Thread.current.status is wrong for killed thread #4705

iaddict · 2017-07-03T13:01:08Z

Environment

Provide at least:

JRuby version: jruby 9.1.12.0 (2.3.3) 2017-06-15 33c6439 Java HotSpot(TM) 64-Bit Server VM 25.112-b16 on 1.8.0_112-b16 [darwin-x86_64]
Operating system and platform: Darwin de01pc11303.global.jhcn.net 16.6.0 Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64

Expected Behavior

A sleeping thread that gets killed should have status 'aborting' and not 'run'.

Actual Behavior

The following code outputs 'run' on jruby, whereas on mri it outputs 'aborting'.

t = Thread.new {
  begin
    sleep
  ensure
    puts Thread.current.status # should output 'aborting' when killed
  end
}
t.kill

Example run on jruby:

irb(main):020:0> t = Thread.new { begin; sleep; ensure; puts Thread.current.status; end }
=> #<Thread:0x79ad8b2f@(irb):20 sleep>
irb(main):021:0> t.kill
run
=> #<Thread:0x79ad8b2f@(irb):20 dead>

Example run on mri ruby:

irb(main):013:0>  t = Thread.new { begin; sleep; ensure; puts Thread.current.status; end }
=> #<Thread:0x007faefc8bad78@(irb):13 run>
irb(main):014:0> t.kill
aborting
=> #<Thread:0x007faefc8bad78@(irb):13 sleep>

This code is a simplified version of ActiveRecord transaction code that fails.
See:

The text was updated successfully, but these errors were encountered:

kares · 2017-07-03T14:26:29Z

did not expect using Thread.current.status to manage transaction state. haven't seen this anywhere else, anyone nows if this is really necessary or just a bad design decision ?

eregon · 2017-07-03T16:20:34Z

Interesting, I recently looked at this in TruffleRuby and could not figure how the user can observe the aborting status.
We should definitely add a spec for this.
Note that you might need a sleep 0.1 before the t.kill in the example above to observe it, as otherwise it could kill the thread before it even reaches the sleep.

iaddict · 2017-07-03T19:29:53Z

Here is a slightly better version of the sample code which uses a Queue to make sure that the thread is sleeping.

q = Queue.new
t = Thread.new { begin; q.push nil; sleep; ensure; puts Thread.current.status; end }
q.pop
t.kill

The status of a dying running or sleeping thread should be 'aborting'. On jruby this is not the case. These spec additions show this issue. There already is an open issue for jruby: jruby/jruby#4705 ``` $ ruby ./jruby/mspec/bin/mspec-run core/thread/stop_spec.rb jruby 9.1.12.0 (2.3.3) 2017-06-15 33c6439 Java HotSpot(TM) 64-Bit Server VM 25.112-b16 on 1.8.0_112-b16 +jit [darwin-x86_64] 1) Thread#status describes a dying running thread FAILED Expected "run" to equal "aborting" ./jruby/ruby-spec/core/thread/stop_spec.rb:60:in `block in (root)' org/jruby/RubyBasicObject.java:1691:in `instance_eval' org/jruby/RubyEnumerable.java:1596:in `all?' org/jruby/RubyFixnum.java:299:in `times' org/jruby/RubyArray.java:1734:in `each' ./jruby/ruby-spec/core/thread/stop_spec.rb:58:in `<main>' org/jruby/RubyKernel.java:979:in `load' org/jruby/RubyBasicObject.java:1691:in `instance_eval' org/jruby/RubyArray.java:1734:in `each' 2) Thread#status describes a dying sleeping thread FAILED Expected "run" to equal "aborting" ./jruby/ruby-spec/core/thread/stop_spec.rb:64:in `block in (root)' org/jruby/RubyBasicObject.java:1691:in `instance_eval' org/jruby/RubyEnumerable.java:1596:in `all?' org/jruby/RubyFixnum.java:299:in `times' org/jruby/RubyArray.java:1734:in `each' ./jruby/ruby-spec/core/thread/stop_spec.rb:58:in `<main>' org/jruby/RubyKernel.java:979:in `load' org/jruby/RubyBasicObject.java:1691:in `instance_eval' org/jruby/RubyArray.java:1734:in `each' [| | ==================100%================== | 00:00:00] 2F 0E Finished in 0.175762 seconds 1 file, 12 examples, 13 expectations, 2 failures, 0 errors, 0 tagged ```

iaddict · 2017-09-11T13:42:54Z

I ran the specs again with the current jruby master branch and they did pass, albeit the above referenced pull request did not catch the issue in its entirety. I created another pull request that properly tests the status of a sleeping killed thread that fails on jruby but passes on mri. See: ruby/spec#462. Thus the problem is still present.

eregon · 2017-09-14T20:29:47Z

That spec is now in jruby and fails as expected:
https://travis-ci.org/jruby/jruby/jobs/275547991

     [exec] Thread#status reports aborting on an externally killed thread that sleeps FAILED
     [exec] Expected "run"
     [exec]  to equal "aborting"
     [exec] 
     [exec] /home/travis/build/jruby/jruby/spec/ruby/core/thread/status_spec.rb:58:in `block in (root)'
     [exec] org/jruby/RubyBasicObject.java:1750:in `instance_eval'
     [exec] org/jruby/RubyEnumerable.java:1704:in `all?'
     [exec] org/jruby/RubyArray.java:1764:in `each'
     [exec] /home/travis/build/jruby/jruby/spec/ruby/core/thread/status_spec.rb:4:in `<main>'
     [exec] org/jruby/RubyKernel.java:986:in `load'
     [exec] org/jruby/RubyBasicObject.java:1750:in `instance_eval'
     [exec] org/jruby/RubyArray.java:1764:in `each'

eregon · 2017-10-05T09:14:06Z

This is still a bug. I tagged the spec so the CI is green in 73b0114

iaddict · 2017-10-05T11:54:29Z

@eregon Does this mean the bug does not get fixed anytime soon? The AR and transactions (mentioned above) will thus persist?

eregon · 2017-10-05T12:44:19Z

I'm not fixing the bug in JRuby, merely updating the specs.
But I guess @headius might take care of it.

…t sleeps (Fixes jruby#4705)

…t sleeps (Fixes #4705)

headius · 2017-11-08T17:08:57Z

Additional commit for this, to actually set aborting status upon kill: 919832e

iaddict mentioned this issue Jul 3, 2017

Jdbc mode rails-sqlserver/activerecord-sqlserver-adapter#599

Closed

iaddict mentioned this issue Jul 4, 2017

Test status of dying threads ruby/spec#456

Closed

iaddict added a commit to iaddict/jruby that referenced this issue Oct 12, 2017

Fix failing spec on Thread#status for an externally killed thread tha…

bacd973

…t sleeps (Fixes jruby#4705)

iaddict mentioned this issue Oct 12, 2017

Fix spec of Thread#status for an externally killed thread that sleeps #4815

Merged

headius closed this as completed in #4815 Oct 12, 2017

headius pushed a commit that referenced this issue Oct 12, 2017

Fix failing spec on Thread#status for an externally killed thread tha…

d335c0c

…t sleeps (Fixes #4705)

headius reopened this Nov 8, 2017

headius added this to the JRuby 9.1.14.0 milestone Nov 8, 2017

headius added the core label Nov 8, 2017

headius closed this as completed Nov 8, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

Thread.current.status is wrong for killed thread #4705

Thread.current.status is wrong for killed thread #4705

iaddict commented Jul 3, 2017

kares commented Jul 3, 2017

eregon commented Jul 3, 2017

iaddict commented Jul 3, 2017 •

edited

Loading

iaddict commented Sep 11, 2017 •

edited

Loading

eregon commented Sep 14, 2017

eregon commented Oct 5, 2017

iaddict commented Oct 5, 2017

eregon commented Oct 5, 2017

headius commented Nov 8, 2017

Thread.current.status is wrong for killed thread #4705

Thread.current.status is wrong for killed thread #4705

Comments

iaddict commented Jul 3, 2017

Environment

Expected Behavior

Actual Behavior

kares commented Jul 3, 2017

eregon commented Jul 3, 2017

iaddict commented Jul 3, 2017 • edited Loading

iaddict commented Sep 11, 2017 • edited Loading

eregon commented Sep 14, 2017

eregon commented Oct 5, 2017

iaddict commented Oct 5, 2017

eregon commented Oct 5, 2017

headius commented Nov 8, 2017

iaddict commented Jul 3, 2017 •

edited

Loading

iaddict commented Sep 11, 2017 •

edited

Loading