Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/lib/libcore.js
Original file line number Diff line number Diff line change
Expand Up @@ -2264,6 +2264,15 @@ addToLibrary({
this.allocated[id] = undefined;
this.freelist.push(id);
}
list() {
var valid_ids = [];
for (var i = 1; i < this.allocated.length; i++) {
if (this.allocated[i] !== undefined) {
valid_ids.push(i);
}
}
return valid_ids;
}
Copy link
Collaborator

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, no problem. I will just revert my last commit. Better play it safe.

},

$wasmTable__docs: '/** @type {WebAssembly.Table} */',
Expand Down
14 changes: 5 additions & 9 deletions src/lib/libwebsocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -409,21 +409,17 @@ var LibraryWebSocket = {
emscripten_websocket_is_supported__proxy: 'sync',
emscripten_websocket_is_supported: () => typeof WebSocket != 'undefined',

emscripten_websocket_deinitialize__deps: ['$WS'],
emscripten_websocket_deinitialize__deps: ['$webSockets', 'emscripten_websocket_delete'],
emscripten_websocket_deinitialize__proxy: 'sync',
emscripten_websocket_deinitialize__deps: ['emscripten_websocket_delete'],
emscripten_websocket_deinitialize: () => {
#if WEBSOCKET_DEBUG
dbg('emscripten_websocket_deinitialize()');
#endif
for (var i in WS.sockets) {
var socket = WS.sockets[i];
if (socket) {
socket.close();
_emscripten_websocket_delete(i);
}
for (let id of webSockets.list()) {
let socket = webSockets.get(id);
socket.close();
_emscripten_websocket_delete(id);
}
WS.sockets = [];
}
}

Expand Down
1 change: 1 addition & 0 deletions test/test_sockets.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ def test_nodejs_sockets_echo_subprotocol_runtime(self):
@parameterized({
'': [[]],
'shared': [['-sSHARED_MEMORY']],
'deinitialize': [['-DTEST_EMSCRIPTEN_WEBSOCKET_DEINITIALIZE']],
})
@requires_dev_dependency('ws')
def test_websocket_send(self, args):
Expand Down
32 changes: 31 additions & 1 deletion test/websocket/test_websocket_send.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand All @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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. #ifdef TEST_EMSCRIPTEN_WEBSOCKET_DEINITIALIZE)

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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The 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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the existing version of the test, emscripten_websocket_close() is used to close individual socket and we are entering the WebSocketClose() callback; and the test ends when both sockets have been closed.

My version of the test uses emscripten_websocket_deinitialize() instead; so WebSocketClose() is never called and I need a way to know when to deinitialize.
I introduced the binary_received variable to be able to know when both sockets have received the binary message so that I can deinitialize.
Testing this variable in the other case is not necessary as they are close individually, so we know we are done when both sockets are closed.

I will try to clarify things in a new commit.

// 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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need force exit here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I am not sure how it works.
I saw that it was used above in WebSocketClose() and I interpreted that at meaning "the test is completed, exit with result code 0 (success)", so I did the same. Since my version of the test never enters WebSocketClose(), I force exit here.
As far as I understand, using emscripten_force_exit() is needed because emscripten_exit_with_live_runtime() is called in main(); otherwise the program would continue to run ?

}
#endif // TEST_EMSCRIPTEN_WEBSOCKET_DEINITIALIZE

return 0;
}

Expand Down