Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permit thread state SUSPENDED to go to STOPPED #53

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

benoittgt
Copy link
Contributor

@benoittgt benoittgt commented Jan 28, 2024

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 rails/rails#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: speedshop/threadbench@7e992f2

Otherwise you can have exception like :

```
Assertion Failed: vernier.cc:857:set_state:state == State::RUNNING || state == State::STARTED
```
@casperisfine
Copy link
Contributor

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
Copy link
Contributor Author

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

@jhawthorn jhawthorn merged commit 4f1d583 into jhawthorn:main Jan 30, 2024
7 checks passed
@jhawthorn
Copy link
Owner

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
Copy link
Contributor Author

@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.

@benoittgt benoittgt deleted the permit-suspended branch January 31, 2024 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants