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
[BUG] Segmentation fault at 0x00007fa2dba4b000 #266
Comments
|
Is this reproducible? Could I see the multiclient code? |
Yes, use this tag: https://github.com/socketry/async-websocket/blob/v0.13.0/examples/chat/multi-client.rb Enable the profiling code that's commented out. In another tab, run |
Could you reproduce the issue? |
Yes. But annoyingly not in a debugger yet. |
Ok, finally got back to this but unfortunately I can no longer reproduce it. This is on Fedora 31. |
Could you try with ruby-prof 1.3? |
Ping. |
@cfis still reproducible as per above, here is backtrace:
|
Bummer. Thanks for checking. |
Ok, finally had a chance to look at this again. I can now reproduce the issue and after a fair bit of digging realized its caused by the merge fiber code. Its causing ruby-prof to mix the stacks of two different fibers, causing ruby-prof's internal stack to grow and grow until it crashes. Next, need to work out a solution. |
Great effort figuring out what is going wrong. |
Ok, my conclusion is merge_fibers is a bad idea. As you know, Ruby executes code via fibers, with there being one or more fibers per thread, with each fiber maintaining its own call stack. By default ruby-prof maintains its own internal call stack per fiber. The way that merge_fibers works is it instead tells ruby-prof to maintain a call stack per thread. The point of merge_fibers it to make result more readable, but even in the simplest cases the results aren't correct. For example if you profile this method:
The with merge_fiber false you'll get this (hopefully correct) result:
But if you merge fibers, you'll get this incorrect result:
Notice that now ruby-prof thinks the calls are recursive (they are not) and resume calls yield (it does not, its two different fibers). So the obvious solution is take out merge fiber functionality. I am sympathetic to the problem that makes results harder to read, but that's better than incorrect results (or in real world cases crashes). If you look at the fiber_test.rb cases, and really look at the results, you'll also see they are incorrect. Perhaps there is a way to merge the results together in Ruby after profiling is done? |
That would make sense to me. The point is, some users will want to merge threads, fibers, etc. They don't care how the function was called, just that it was, and how much of an impact it had on the performance of the application. Falcon could literally spawn 10,000 or more fibers. So, I assume you will print one report per fiber? But that's not enough data to draw any meaningful conclusions. |
Well 1 report, but 10,000 sections. So not helpul. In the case of Falcon, are those fibers all workers that start with the same top level method and then will end up calling different methods depending on the incoming request? So the same idea as a thread pool in other webservers? If that's the case, where all the fibers (or threads) have the same root method, then it seems possible to to merge the various call trees together. So maybe some flag called merge_results that would do that? |
That's often the case. |
It only happens with RubyProf running.
There seem to be issues running with RubyProf.
I'll try to figure out what is going wrong.
The text was updated successfully, but these errors were encountered: