jhawthorn / vernier

📏 next generation CRuby profiler
https://vernier.prof/
MIT License
718 stars 15 forks source link

Permit thread state SUSPENDED to go to STOPPED #53

Closed benoittgt closed 5 months ago

benoittgt commented 5 months ago

TLDR; Otherwise you can have exception like :

Assertion Failed: vernier.cc:857:set_state:state == State::RUNNING || state == State::STARTED

I wanted to plug Vernier to the benchmark mentioned here https://github.com/rails/rails/issues/50450. While trying to plug it, I was stop by an exception from Vernier. I modified the code to permit this handling of state and I was able to get a result.

It is maybe a mistake from when I tried to setup Vernier on this project. It can be seen here.

I am not sure we want to permit this, or instead raise and error for invalid code ?

Full stacktrace can be see here: https://github.com/speedshop/threadbench/commit/7e992f2774ab197640c02a14405eee635939a45f

casperisfine commented 5 months ago

STOPPED in Vernier corresponds to EXITED?

If so, then 👍 as it's part of the spec that SUSPENDED -> EXITED is possible: https://github.com/ruby/ruby/blob/8bff7e996cf65159b4ed7b55c284de6651b7e637/test/-ext-/thread/helper.rb#L42-L45

benoittgt commented 5 months ago

Thanks for pointing this code. I was looking for reference to what is "STOPPED" in Ruby codebase.

jhawthorn commented 5 months ago

Thank you! I'll try to cut a release soon

Do you happen to know the code which causes this pattern. It would be great to have a test case for it.

benoittgt commented 5 months ago

@jhawthorn not yet but I will be interested to dig in. Because it was during a Puma benchmark I am curious to see when it was.