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

Exceptions in on_message and on_data callbacks are suppressed #26

Closed
haukot opened this issue Mar 9, 2018 · 3 comments
Closed

Exceptions in on_message and on_data callbacks are suppressed #26

haukot opened this issue Mar 9, 2018 · 3 comments
Labels

Comments

@haukot
Copy link

haukot commented Mar 9, 2018

Hi,
I found that iodine doesn't handle exceptions raised in on_message and on_data callbacks.

Example app:

class MyChatroom
  def on_message(data)
    raise "SomeError"
  end
end

Iodine::Rack.app = Proc.new do |env|
  unless env['upgrade.websocket?']
    return [404, { 'Content-Type' => 'text/plain' }, ['Not Found']]
  end

  env['upgrade.websocket'] = MyChatroom.new

  [-1, {}, []]
end

Iodine.start

When an exception raised the whole thread gets stuck and no messages are written to the log.
The messages are shown when the server stops, or when all available threads terminate (and the server crashes).

I guess it's because there is direct Ruby C API function call instead of RubyCaller call
https://github.com/boazsegev/iodine/blob/master/ext/iodine/iodine_websockets.c#L683
there should probably be something like this:

RubyCaller.call_arg(handler, iodine_on_message_func_id, 1, &buffer);
@boazsegev
Copy link
Owner

@haukot ,

Thank you very much for opening this issue!

I guess it's because there is direct Ruby C API function call instead of RubyCaller call

Yes, you're correct.

However, RubyCaller wouldn't help to fix this issue, because the issue is in the RubyCaller implementation.

The best solution is to redesign the RubyCaller implementation so it always protects against exceptions, so it will wrap the code and protect it at this point.

At the moment, RubyCaller only protects against exceptions if the GVL state needs to be updated when entering Ruby code, not when entering C code.

I wrote a fix in the 0.5.0 version branch, I will see if I can back port it to the 0.4.x version branch.

@boazsegev boazsegev added the bug label Mar 10, 2018
boazsegev pushed a commit that referenced this issue Mar 10, 2018
…ven on C functions

@haukot opened issue #26 that exposed an exception handling issue,
where exceptions weren’t handled when the GVL was entered using
`RubyCaller.call_c` rather than a Ruby `RubyCaller.call`.

This redesigned implementation should solve the issue.
@boazsegev
Copy link
Owner

I released version 0.4.17 which should solve the issue.

Again, thank you very much for detecting this issue 🙏🏻

@haukot
Copy link
Author

haukot commented Mar 10, 2018

Thank you for so fast fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants