https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112018-07-26T06:39:03ZRuby Issue Tracking SystemRuby master - Bug #14939: [PATCH] cont.c (ec_switch): prevent delayed/missed trap interrupt racehttps://bugs.ruby-lang.org/issues/14939?journal_id=731412018-07-26T06:39:03Zko1 (Koichi Sasada)
<ul></ul><p>Good point. Seems good. Thank you!</p>
<p>BTW, I forget that why the trap handler is limited to main thread.</p>
<pre><code>trap(:INT) do
p Thread.current
end
Thread.new{
sleep 1
Process.kill :INT, $$
p [:child, Thread.current]
}.join
sleep 10
p [:main, Thread.current]
__END__
[:child, #<Thread:0x00000125801a3028@t.rb:5 run>]
#<Thread:0x00000125801e9c80 run>
# after 10 seconds
[:main, #<Thread:0x00000125801e9c80 run>]
</code></pre>
<p>Can we relax this limitation?</p> Ruby master - Bug #14939: [PATCH] cont.c (ec_switch): prevent delayed/missed trap interrupt racehttps://bugs.ruby-lang.org/issues/14939?journal_id=731422018-07-26T06:40:23Zko1 (Koichi Sasada)
<ul></ul><p>Oops, sorry forget about <code>sleep</code> lines.</p> Ruby master - Bug #14939: [PATCH] cont.c (ec_switch): prevent delayed/missed trap interrupt racehttps://bugs.ruby-lang.org/issues/14939?journal_id=731442018-07-26T08:30:33Znormalperson (Eric Wong)normalperson@yhbt.net
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Closed</i></li></ul><p>Applied in changeset trunk|r64062.</p>
<hr>
<p>cont.c (ec_switch): prevent delayed/missed trap interrupt race</p>
<p>timer-thread may set trap interrupt with rb_threadptr_check_signal<br>
at any time independent of GVL. This means timer-thread may set<br>
the trap interrupt flag on the previous execution context; causing<br>
the flag to be unnoticed until a future ec switch (or lost<br>
completely if the ec is done).</p>
<p>Note: I avoid relying on th->interrupt_lock here and use<br>
atomics because we won't be able to rely on it for proposed lazy<br>
timer-thread [Misc <a class="issue tracker-5 status-5 priority-4 priority-default closed" title="Misc: [PATCH] thread_pthread: lazy-spawn timer-thread only on contention (Closed)" href="https://bugs.ruby-lang.org/issues/14937">#14937</a>].</p>
<p>This regression affects Ruby 2.5 as it was introduced by moving<br>
interrupt_flag to `ec' which is an unstable pointer. Ruby <= 2.4<br>
was unaffected because vm->main_thread->interrupt_flag never<br>
changed.</p>
<p><a href="/issues/14939">[ruby-core:88119]</a> [Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: [PATCH] cont.c (ec_switch): prevent delayed/missed trap interrupt race (Closed)" href="https://bugs.ruby-lang.org/issues/14939">#14939</a>]</p> Ruby master - Bug #14939: [PATCH] cont.c (ec_switch): prevent delayed/missed trap interrupt racehttps://bugs.ruby-lang.org/issues/14939?journal_id=731452018-07-26T08:42:38Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p><a href="mailto:ko1@atdot.net" class="email">ko1@atdot.net</a> wrote:</p>
<blockquote>
<p>Good point. Seems good. Thank you!</p>
</blockquote>
<p>OK, commited as r64062</p>
<blockquote>
<p>BTW, I forget that why the trap handler is limited to main thread.</p>
</blockquote>
<p>I can think of at least three reasons:</p>
<ol>
<li>
<p>Similar to this bug: other threads may exit soon,<br>
main thread is long-lived so interrupt won't get lost</p>
</li>
<li>
<p>Typical trap handlers are established at program startup,<br>
before other threads exist, so maybe some are reliant on<br>
Thread.current[] values inside trap (perhaps unknowingly<br>
through libraries).</p>
</li>
<li>
<p>Concurrency problem. Mutex isn't usable inside trap,<br>
but there may be instances of reentrancy-safe code which<br>
can't be made thread-safe.</p>
</li>
</ol>
<blockquote>
<p>Can we relax this limitation?</p>
</blockquote>
<p>I don't think it's necessary; anybody who relies on trap will<br>
likely structure main thread to deal with it (e.g. not block on<br>
slow NFS IO#read :P). We can workaround 1), but I expect<br>
compatibility and safety problems from 2) and 3).</p> Ruby master - Bug #14939: [PATCH] cont.c (ec_switch): prevent delayed/missed trap interrupt racehttps://bugs.ruby-lang.org/issues/14939?journal_id=744152018-10-11T14:58:34Znagachika (Tomoyuki Chikanaga)nagachika00@gmail.com
<ul><li><strong>Backport</strong> changed from <i>2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: REQUIRED</i> to <i>2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: DONE</i></li></ul><p>ruby_2_5 r64999 merged revision(s) 64062.</p>