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

Fix hanging of WebSocket::stop() waiting for its thread to join #481

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

dacap
Copy link
Contributor

@dacap dacap commented Aug 23, 2023

This might happen in some special cases where WebSocketTransport::sendOnSocket() fails, closes the socket, and set the ready state to CLOSED, but WebSocket::run() stills wait the interrupt event to happen.

Possibly related to #474.

@dacap
Copy link
Contributor Author

dacap commented Aug 23, 2023

I've just updated the PR to match the if (...) { ... } style.

This might happen in some special cases where
WebSocketTransport::sendOnSocket() fails, closes the socket, and set
the ready state to CLOSED, but WebSocket::run() stills wait the
interrupt event to happen.

Possibly related to machinezone#474
@dacap dacap reopened this Aug 29, 2023
@dacap
Copy link
Contributor Author

dacap commented Aug 29, 2023

Finally updated this PR as a specific test (IXWebSocketTestConnectionDisconnection) was failing.

Now all tests pass.

dacap added a commit to aseprite/aseprite that referenced this pull request Aug 29, 2023
This can happen using Pribambase extension when we close Aseprite in
some strange way (e.g. killing the process), and the connection is not
closed correctly. It looks like the Blender addon is in an invalid
state and the data cannot be sent any more, but without this patch
Aseprite just hangs/crashes if we retry sending data, but with this
fix we can receive/show that an error happened sending data.

Related to: machinezone/IXWebSocket#481
@bsergean
Copy link
Collaborator

bsergean commented Sep 1, 2023

looking good ci wise, re-ran them.

@bsergean bsergean merged commit ef57e3a into machinezone:master Sep 1, 2023
16 checks passed
@bsergean
Copy link
Collaborator

bsergean commented Sep 1, 2023

thanks @dacap

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.

2 participants