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

Calling websocket->close() and websocket->write() from another thread #1153

Closed
ludekvodicka opened this issue Dec 23, 2020 · 11 comments
Closed
Labels

Comments

@ludekvodicka
Copy link

Sorry for bothering again. I have another crash on Win64 but I'm not sure if this is a bug or wrong library usage.

Should it be safe to call websocket->close() from another thread (or websocket->write()) ?

Occasionally the application is crashing after calling websocket->close() from the main application thread (when simulating manual client disconnection in our tests).

It's because s->context is 0xddddddddddd during TopicTree::unsubscribeAll()

But I'm not sure if this is a bug or it's caused by calling it from the other thread because this code https://github.com/uNetworking/uSockets/blob/master/src/socket.c#L65 doesn't seem to thread-safe.

Is there any list of methods that are safe to call from another thread?

And is there any "writeable" callback in the uWebsockets thread where is it possible to run these not thread-safe methods? I read about us_timer_set which should be probably utilized too (https://github.com/uNetworking/uWebSockets/blob/master/examples/UpgradeAsync.cpp)

Thank you

@ludekvodicka ludekvodicka changed the title Is it thread-safe to call Calling websocket->close() and websocket->write() from another thread Dec 23, 2020
@ludekvodicka
Copy link
Author

I just find that also calling us_listen_socket_close from the main thread is occasionally crashing the app.

devenv_2020-12-23_15-50-29

ApplicationFrameHost_2020-12-23_15-52-25

@hst-m
Copy link
Contributor

hst-m commented Dec 23, 2020

Have you read user manual regarding threads? https://github.com/uNetworking/uWebSockets/blob/master/misc/READMORE.md

@ludekvodicka
Copy link
Author

I read it several times but based on your reply I probably missed something.

I know a whole uWS::App has to run in own thread. But I didn't find any example/mention in the docs how to invoke commands (finalize loop, send data,...) from other threads.

I'm aware that all these issues are caused probably by missing synchronization, but I didn't find any way how to invoke it from the uWS::App thread. This is the reason why I'm posting it here.

Should I use a timer for such cases? Or is there any "Writeable" callback where I can execute it?

Thanks for pointing me to any example/docs.

@ludekvodicka
Copy link
Author

ludekvodicka commented Dec 23, 2020

It seems I found the solution in uWS::Loop::get() and uWS::Loop::defer(), but I'm not sure if this is currently the recommended way, because there is no mention of this anywhere in the documentation or examples.

@ghost
Copy link

ghost commented Dec 23, 2020

Loop defer is the only thread safe function.

@ludekvodicka
Copy link
Author

ludekvodicka commented Dec 24, 2020

Thanks for the confirmation and for the great library.

If I can suggest, it would be probably good to have any example or mention about defer in the documentation.

@jrch2k20
Copy link

jrch2k20 commented Jan 5, 2021

Yeah, i would appreciate an example on how to use uWS::SSLApp::Publish from another thread.

@ghost
Copy link

ghost commented Jan 5, 2021

If you search in Discussions you'll find 5 answers of this

@jrch2k20
Copy link

jrch2k20 commented Jan 5, 2021

I literally just got mind blown that there is a discussion tab on github LOL, sry i'll check there now.

100% my bad

@ghost
Copy link

ghost commented Jan 5, 2021

Hehe np, maybe I should link to it from the readme. It's a new tab

@jrch2k20
Copy link

jrch2k20 commented Jan 5, 2021

yeah i think would be nice, good stuff in there and yeah defer and publish working like a charm now ;)

Awesome work and tyvm for your time

@ghost ghost closed this as completed Jan 11, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants