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

Crash when unwrapping NIOLoopBound from responder without sufficient EL checks #3078

Closed
Lukasa opened this issue Oct 2, 2023 · 0 comments · Fixed by #3081
Closed

Crash when unwrapping NIOLoopBound from responder without sufficient EL checks #3078

Lukasa opened this issue Oct 2, 2023 · 0 comments · Fixed by #3081
Labels
bug Something isn't working

Comments

@Lukasa
Copy link
Contributor

Lukasa commented Oct 2, 2023

In #3057 many changes got made like the ones in HTTPServerHandler that look fundamentally like this:

Screenshot 2023-10-02 at 08 17 46

This code is not sound. Specifically, the code fails to ensure that the future that is returned from respond(to:) is bound to the same event loop as the one that this request came in on. This can happen in lots of ways, but it's most common when interacting with databases or other tools, which can end up giving futures that resolve on other threads. This can lead to a crash unwrapping the NIOLoopBound.

These changes need to be accompanied with a call to hop(to:) on the ELF to ensure that you are on the correct thread before unwrapping the loop-bound.

@Lukasa Lukasa added the bug Something isn't working label Oct 2, 2023
@0xTim 0xTim linked a pull request Oct 5, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant