patches: carry upstream PR 68 instead of our narrower fix #106

Merged

yuris merged 1 commit from adopt-upstream-patch into main

2026-08-22 03:58:24 +00:00

yuris commented

2026-08-22 03:53:12 +00:00

Owner

patches/extension-websocket-html5-subprotocol.patch now carries upstream PR #68 verbatim instead of our narrower fix. The diff body is byte-identical to 68.diff; only the prose header is ours.

Why theirs is better

Same root cause. 4.2.3 starts the platform connection inside CreateConnection(), which runs before LuaConnect() assigns m_ConnectTimeout, m_CustomHeaders, m_Protocol and m_Callback, and conn was memset to zero. #68 moves both platform starts into StartConnection() and calls it at the end of LuaConnect, once every field is set and the connection is registered. Ours moved only the emscripten call, and only as far as the two strdups it needed — so it still started the socket before m_Callback existed.

Ours left the native half in place. CreateConnectionWslay spawns ConnectionWorker immediately, and that thread reads m_ConnectTimeout via CheckConnectTimeout (t >= conn->m_ConnectTimeout, true for any t while the field is 0) before it ever reaches m_Protocol in handshake.cpp. Two strdups beat a TCP connect essentially always, which is why this was invisible.

Made it visible by injecting a 300 ms sleep after CreateConnection() and building three headless x86_64-linux bundles against a live server:

build	result
4.2.3 as released	connects, `PHASE1 OK`
4.2.3 + 300 ms delay	never opens a socket, retries forever
#68 + the same delay	connects, `PHASE1 OK`

A TCP tee in front of the port confirms the failing build sends no handshake at all — it dies on the timeout check before it could offer a subprotocol.

Our old patch header said "Native builds are unaffected: wslay writes the handshake later, from the update loop." It writes it later from a thread, which is not the same guarantee. That claim had also reached Installation.md; both are corrected here, and the wiki now points at #68.

Verification

HTML5 conformance with the new patch in place, both wires:

[handshake] -> subprotocol=v2.bsatn.spacetimedb   <- 101 Switching Protocols   PASS
[handshake] -> subprotocol=v1.json.spacetimedb   <- 101 Switching Protocols   PASS

Seven phases each, across a server restart and a page reload. patch -p1 applies clean to 4.2.3.

Adopting it before it merges means the harness tests the code upstream will actually release. When a release carries it, this file, the apply step in html5-conformance.sh and the remote-dependency instruction in the install docs all go together — that teardown is written into the patch header.

`patches/extension-websocket-html5-subprotocol.patch` now carries [upstream PR #68](https://github.com/defold/extension-websocket/pull/68) verbatim instead of our narrower fix. The diff body is byte-identical to `68.diff`; only the prose header is ours. ## Why theirs is better Same root cause. 4.2.3 starts the platform connection inside `CreateConnection()`, which runs before `LuaConnect()` assigns `m_ConnectTimeout`, `m_CustomHeaders`, `m_Protocol` and `m_Callback`, and `conn` was memset to zero. #68 moves both platform starts into `StartConnection()` and calls it at the end of `LuaConnect`, once every field is set and the connection is registered. Ours moved only the emscripten call, and only as far as the two strdups it needed — so it still started the socket before `m_Callback` existed. **Ours left the native half in place.** `CreateConnectionWslay` spawns `ConnectionWorker` immediately, and that thread reads `m_ConnectTimeout` via `CheckConnectTimeout` (`t >= conn->m_ConnectTimeout`, true for any `t` while the field is 0) before it ever reaches `m_Protocol` in `handshake.cpp`. Two strdups beat a TCP connect essentially always, which is why this was invisible. Made it visible by injecting a 300 ms sleep after `CreateConnection()` and building three headless x86_64-linux bundles against a live server: | build | result | |---|---| | 4.2.3 as released | connects, `PHASE1 OK` | | 4.2.3 + 300 ms delay | **never opens a socket**, retries forever | | #68 + the same delay | connects, `PHASE1 OK` | A TCP tee in front of the port confirms the failing build sends no handshake at all — it dies on the timeout check before it could offer a subprotocol. Our old patch header said "Native builds are unaffected: wslay writes the handshake later, from the update loop." It writes it later from a **thread**, which is not the same guarantee. That claim had also reached `Installation.md`; both are corrected here, and the wiki now points at #68. ## Verification HTML5 conformance with the new patch in place, both wires: ``` [handshake] -> subprotocol=v2.bsatn.spacetimedb <- 101 Switching Protocols PASS [handshake] -> subprotocol=v1.json.spacetimedb <- 101 Switching Protocols PASS ``` Seven phases each, across a server restart and a page reload. `patch -p1` applies clean to 4.2.3. Adopting it before it merges means the harness tests the code upstream will actually release. When a release carries it, this file, the apply step in `html5-conformance.sh` and the remote-dependency instruction in the install docs all go together — that teardown is written into the patch header.

yuris added 1 commit

2026-08-22 03:53:13 +00:00

patches: carry upstream PR 68 instead of our narrower fix

ci / ci (pull_request) Successful in 1m27s

Details

conformance / conformance (v1) (pull_request) Successful in 2m8s

Details

conformance / conformance (v2) (pull_request) Successful in 2m33s

Details

595394582f

Same root cause, wider fix, and it is the code that will actually ship. 4.2.3
starts the platform connection inside CreateConnection(), before LuaConnect()
assigns m_ConnectTimeout, m_CustomHeaders, m_Protocol and m_Callback; PR 68
moves both platform starts into StartConnection() and calls it once every field
is set and the connection is registered. Ours moved only the emscripten call,
and only as far as the two strdups it needed.

What ours left behind was the native half. CreateConnectionWslay spawns
ConnectionWorker immediately, and that thread reads m_ConnectTimeout through
CheckConnectTimeout - 't >= 0' is true for any t - before it ever reaches
m_Protocol in handshake.cpp. Two strdups beat a TCP connect essentially always,
so nobody has seen it. Forced the other way with a 300ms sleep after
CreateConnection, the unpatched build never opens a socket at all while the
patched one connects normally. Our patch header claimed native was unaffected
because 'wslay writes the handshake later, from the update loop'; it writes it
later from a THREAD, which is not the same guarantee. The install docs carried
that claim too and no longer do.

Verified with the new patch in place: HTML5 offers the subprotocol and gets 101
on both wires, seven phases each across a server restart and a page reload.

Adopting it before it merges means the harness now tests the code upstream will
release. When a release carries it, this file, the apply step in
html5-conformance.sh and the remote-dependency instruction all go together.

yuris merged commit 732068a223 into main

2026-08-22 03:58:24 +00:00

yuris referenced this pull request from a commit

2026-08-22 03:58:25 +00:00

patches: carry upstream PR 68 instead of our narrower fix (#106)