move UDP sockets to worker thread #306
Conversation
manekinekko
changed the title
|
40/41 test programs passing. The remaining test progran is failing because of this. I had to skip some unit tests because we need to stub some of the socket async operations triggered in the worker thread. |
manekinekko
force-pushed
the
chore/wasi-sockets-udp-worker-thread
branch
from
0cdbb96 to
ec40864
Compare
| export function getSocketOrThrow(socketId) { | ||
| const socket = openedSockets.get(socketId); | ||
| if (!socket) throw "invalid-state"; | ||
| return socket; | ||
| } | ||
|
|
||
| export function getSocketByPort(port) { | ||
| return Array.from(openedSockets.values()).find((socket) => socket.address().port === port); | ||
| } | ||
|
|
||
| export function getBoundSockets(socketId) { | ||
| return Array.from(openedSockets.entries()) | ||
| .filter(([id, _socket]) => id !== socketId) // exclude source socket | ||
| .map(([_id, socket]) => socket.address()); | ||
| } | ||
|
|
||
| export function dequeueReceivedSocketDatagram(socketInfo, maxResults) { | ||
| const dgrams = queuedReceivedSocketDatagrams.get(`PORT:${socketInfo.port}`).splice(0, Number(maxResults)); | ||
| return dgrams; | ||
| } | ||
| export function enqueueReceivedSocketDatagram(socketInfo, { data, rinfo }) { | ||
| const key = `PORT:${socketInfo.port}`; | ||
| const chunk = { | ||
| data, | ||
| rinfo, // sender/remote socket info (source) | ||
| socketInfo, // receiver socket info (targeted socket) | ||
| }; | ||
|
|
||
| // create new queue if not exists | ||
| if (!queuedReceivedSocketDatagrams.has(key)) { | ||
| queuedReceivedSocketDatagrams.set(key, []); | ||
| } | ||
|
|
||
| // append to queue | ||
| const queue = queuedReceivedSocketDatagrams.get(key); | ||
| queue.push(chunk); | ||
| } |
There was a problem hiding this comment.
Can we move all of these into worker-socket-udp.js.
There was a problem hiding this comment.
Correct. Will do.
| return createFuture(createUdpSocket(addressFamily, reuseAddr)); | ||
| } | ||
|
|
||
| case SOCKET_UDP_BIND: { |
There was a problem hiding this comment.
For any UDP call that is longer than a couple of lines, like this one, can we abstract it out into a function and instead call that function import from the worker-socket-udp.js file.
| } | ||
| } | ||
|
|
||
| case SOCKET_UDP_SEND: { |
| const exceptionInfo = {}; | ||
| const value = this.#socket.bufferSize(0, BufferSizeFlags.SO_RCVBUF, exceptionInfo); | ||
| // TODO: should we throw if the socket is not bound? | ||
| // assert(this[symbolSocketState].isBound === false, "invalid-state"); |
There was a problem hiding this comment.
Perhaps raise this on the sockets spec as a question?
| test("tcp.connect(): should connect to a valid ipv4 address and port=0", async () => { | ||
| // test passing but blocked on socket.connect(). | ||
| // TODO: figure out how to mock handle.connect() | ||
| test.skip("tcp.connect(): should connect to a valid ipv4 address and port=0", async () => { |
There was a problem hiding this comment.
For these tests that can't be easily mocked, it would be better to remove them from this unit test suite and instead just rely on the integration test coverage since that is sufficient at this point.
|
I've gone ahead with the merge for now - let's follow up on the review feedback in follow-up PRs. |
|
Great! Thanks @guybedford I will then move my local pending changes to a different PR. |
manekinekko
changed the title
preview2_udp_bindpreview2_udp_connectpreview2_udp_sample_applicationpreview2_udp_sockoptspreview2_udp_states