Bug #15992: An exception breaks monitor state and cause deadlock

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 2.5: DONE, 2.6: DONE

[naruse (Yui NARUSE)]

lib/monitor.rb provides Monitor.
But its state handling is weak for interrupts caused by Thread.kill for example timeout libraries.

Timeout exception may happen everywhere. If it raised when the thread is executing

  def mon_exit
    mon_check_owner
    @mon_count -=1
    if @mon_count == 0
      @mon_owner = nil
      # HERE!!!
      @mon_mutex.unlock
    end
  end

It breaks the state of the monitor and it causes deadlock.

[naruse (Yui NARUSE)]

• Status changed from Open to Closed

Applied in changeset git|f91879a7b548284c93743168acfd11e3d2aeefac.

---

handle_interrupt to defend monitor state [Bug #15992]

If an exception is raised from another thread for example Timeout
and this thread is just after mon_exit's @mon_owner = nil,
the exception breaks the state of MonitorMixin. To prevent that situation,
it need to block interruption in mon_enter and mon_exit.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.5: REQUIRED, 2.6: REQUIRED to 2.5: REQUIRED, 2.6: DONE

ruby_2_6 r67742 merged revision(s) f91879a7b548284c93743168acfd11e3d2aeefac.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.5: REQUIRED, 2.6: DONE to 2.5: REQUIRED, 2.6: REQUIRED

9557069299ac3b96691040a541afa65761a724ad is a follow up commit to ease memory consumption impact of f91879a7b548284c93743168acfd11e3d2aeefac. I think it should be backported too.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.5: REQUIRED, 2.6: REQUIRED to 2.5: REQUIRED, 2.6: DONE

ruby_2_6 r67749 merged revision(s) 9557069299ac3b96691040a541afa65761a724ad.

[usa (Usaku NAKAMURA)]

• Backport changed from 2.5: REQUIRED, 2.6: DONE to 2.5: DONE, 2.6: DONE

ruby_2_5 r67774 merged revision(s) f91879a7b548284c93743168acfd11e3d2aeefac,9557069299ac3b96691040a541afa65761a724ad.

[Eregon (Benoit Daloze)]

Should mon_enter and mon_exit use Thread.handle_interrupt inside their method definitions, so they are safe when called directly too?

Thread.handle_interrupt(Exception => :never) seems too much for mon_enter, I think Thread.handle_interrupt(Exception => :on_blocking) would be better.
Otherwise, it's not possible to interrupt a thread trying to get a contended monitor with Thread#raise:

ruby -rmonitor -e 'o=Object.new.extend(MonitorMixin); Thread.new{ o.mon_enter; sleep }; sleep 0.1; Thread.new{sleep 3; p :kill; Thread.main.raise "bye"}; p o.synchronize { p :in; sleep }'

Never exits on trunk, but it does exit on 2.6.2 and before. Even Thread#kill does not help.

Ctrl+C does interrupt it, because signal traps are not yet supported by handle_interrupt, but that might change.