Project

General

Profile

Feature #15002

[PATCH] thread.c (sleep_*): reduce the effect of spurious interrupts

Added by normalperson (Eric Wong) about 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Target version:
-
[ruby-core:88510]

Description

Spurious interrupts from SIGCHLD cause Mutex#sleep (via
ConditionVariable#wait) to return early and breaks some use
cases.  Since these are outside the programs's control with
MJIT, we will only consider pending interrupts (e.g. those
from Thread#run) and signals which cause a Ruby-level Signal.trap
handler to fire as "spurious" wakeups.

This fixes all the Net::IMAP tests in test-all for me with --jit-wait

Remaining test-all failures with --jit-wait after this patch:

>   1) Failure:
> TestThreadQueue#test_queue_close_multi_multi [/ruby/test/ruby/test_thread_queue.rb:526]:
> no threads running

Seems like the test is too timing-sensitive for producer thread lifetime.

>   2) Failure:
> TestIO#test_recycled_fd_close [/ruby/test/ruby/test_io.rb:3804]:
> Expected /stream closed/ to match "closed stream".

Not sure if solvable with MJIT, the difference is minor and won't
affect real-world tests.  We should probably loosen the test to allow
either "stream closed" or "closed stream"

>   3) Failure:
> TestRubyOptimization#test_tailcall_condition_block [/ruby/test/ruby/test_optimization.rb:439]:
> [ruby-core:78015] [Bug #12905]: 10079 / 20158 stack levels.
> Exception raised:
> <#<SystemStackError: stack level too deep>>.

Haven't investigated (I think k0kubun would know better)


Files

Updated by k0kubun (Takashi Kokubun) about 2 years ago

  • Assignee changed from k0kubun (Takashi Kokubun) to normalperson (Eric Wong)

Thanks to deal with it. Actually test-all with --jit-wait is running successfully on my Wercker CI and ko1's trunk-mjit-wait for now http://ci.rvm.jp/results/trunk-mjit-wait@silicon-docker, but as long as it passes the tests on your environment, it looks good to merge the change to deal with the spurious interrupts by SIGCHLD from MJIT.

Haven't investigated (I think k0kubun would know better)

I haven't investigated yet either.

#2

Updated by normalperson (Eric Wong) about 2 years ago

  • Status changed from Open to Closed

Applied in changeset trunk|r64444.


thread.c (sleep_*): reduce the effect of spurious interrupts

Spurious interrupts from SIGCHLD cause Mutex#sleep (via
ConditionVariable#wait) to return early and breaks some use
cases. Since these are outside the programs's control with
MJIT, we will only consider pending interrupts (e.g. those
from Thread#run) and signals which cause a Ruby-level Signal.trap
handler to fire as "spurious" wakeups.

[ruby-core:88537] [Feature #15002]

Updated by normalperson (Eric Wong) about 2 years ago

takashikkbn@gmail.com wrote:

Thanks to deal with it. Actually test-all with --jit-wait is
running successfully on my Wercker CI and ko1's
trunk-mjit-wait for now
http://ci.rvm.jp/results/trunk-mjit-wait@silicon-docker, but
as long as it passes the tests on your environment, it looks
good to merge the change to deal with the spurious interrupts
by SIGCHLD from MJIT.

OK, r64444 is committed. It should fix spurious wakeups from
the new ConditionVariable#wait specs

TestThreadQueue#test_queue_close_multi_multi [/ruby/test/ruby/test_thread_queue.rb:526]:
no threads running

TestIO#test_recycled_fd_close [/ruby/test/ruby/test_io.rb:3804]:
Expected /stream closed/ to match "closed stream".

So you guys don't see these two failures under CI?
I got them every time...

3) Failure:
TestRubyOptimization#test_tailcall_condition_block [/ruby/test/ruby/test_optimization.rb:439]:
[ruby-core:78015] [Bug #12905]: 10079 / 20158 stack levels.
Exception raised:
<#>.

This one might be sporadic, I don't think I saw it every time.

Also available in: Atom PDF