-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix emscripten_websocket_deinitialize() #25744
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,11 @@ | |
| #include <emscripten/websocket.h> | ||
| #include <assert.h> | ||
|
|
||
| // This test performs that same server communications using two different | ||
| // This test performs the same server communications using two different | ||
| // sockets. This verifies that multiple sockets are supported simultaneously. | ||
| // Depending on whether TEST_EMSCRIPTEN_WEBSOCKET_DEINITIALIZE is defined, | ||
| // cleanup is either performed using emscripten_websocket_deinitialize() or | ||
| // emscripten_websocket_close() and emscripten_websocket_delete(). | ||
| EMSCRIPTEN_WEBSOCKET_T sock1; | ||
| EMSCRIPTEN_WEBSOCKET_T sock2; | ||
|
|
||
|
|
@@ -44,6 +47,7 @@ bool WebSocketError(int eventType, const EmscriptenWebSocketErrorEvent *e, void | |
| bool WebSocketMessage(int eventType, const EmscriptenWebSocketMessageEvent *e, void *userData) { | ||
| printf("message(socket=%d, eventType=%d, userData=%p data=%p, numBytes=%d, isText=%d)\n", e->socket, eventType, userData, e->data, e->numBytes, e->isText); | ||
| static int text_received = 0; | ||
| static int binary_received = 0; | ||
| assert(e->socket == sock1 || e->socket == sock2); | ||
| if (e->isText) { | ||
| printf("text data: \"%s\"\n", e->data); | ||
|
|
@@ -54,13 +58,39 @@ bool WebSocketMessage(int eventType, const EmscriptenWebSocketMessageEvent *e, v | |
|
|
||
| // We expect to receive the text message before the binary one | ||
| assert(text_received); | ||
| binary_received++; | ||
| printf("binary data:"); | ||
| for (int i = 0; i < e->numBytes; ++i) { | ||
| printf(" %02X", e->data[i]); | ||
| assert(e->data[i] == i); | ||
| } | ||
| printf("\n"); | ||
|
|
||
| #ifndef TEST_EMSCRIPTEN_WEBSOCKET_DEINITIALIZE | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you flip these block around so that this is possive test (i.e. |
||
| emscripten_websocket_close(e->socket, 0, 0); | ||
| // The WebSocket is being closed, but its handle is still valid. | ||
| // It should therefore still be possible to query its state. | ||
| unsigned short ready_state; | ||
| EMSCRIPTEN_RESULT result = emscripten_websocket_get_ready_state(e->socket, &ready_state); | ||
| assert(result == EMSCRIPTEN_RESULT_SUCCESS); | ||
| // https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/readyState | ||
| assert(ready_state == 2); // 2 = CLOSING | ||
| #else | ||
| if (binary_received == 2) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this change here but not in the other block? |
||
| // We successfully received binary data from both websockets. | ||
| // We are done. We can deinitialize and exit. | ||
| emscripten_websocket_deinitialize(); | ||
| // All websocket handles are invalidated. | ||
| // It is no longer possible to query their state. | ||
| unsigned short ready_state; | ||
| EMSCRIPTEN_RESULT result = emscripten_websocket_get_ready_state(e->socket, &ready_state); | ||
| assert(result == EMSCRIPTEN_RESULT_INVALID_TARGET); | ||
| (void)ready_state; | ||
| sock1 = sock2 = 0; | ||
| emscripten_force_exit(0); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need force exit here? |
||
| } | ||
| #endif // TEST_EMSCRIPTEN_WEBSOCKET_DEINITIALIZE | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to add more method to this class I'm afraid because it shared with all the other users.
Maybe just implement this locally in the websocket code?