Rooms are served by a build without WebSocket - #166
Merged
Conversation
The pub/sub core has not known what a connection is since it moved to src/room/, but the build gate still said otherwise: the sources sat behind --enable-websocket, the Room class was guarded, and every room entry point had an #else arm that threw. All of that goes. RoomDeliveryException moves to its own stub and extends HttpServerException: rooms are served where WebSocketException does not exist. This is a BC break and CHANGELOG says so. Two defects only the new configuration could show, both fixed here: getRuntimeStats() reported no room counters without WebSocket, and room_hub_thread_sweep() sat behind the guard, so a thread that subscribed to a room never detached — a leaked mailbox and a live libuv handle at shutdown. The ten connectionless room tests move to tests/phpt/room/ so the new CI leg runs them all, and the leak lane follows them.
Contributor
CoverageTotal lines: 82.24% → 82.33% (+0.09 pp)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The room core stopped knowing what a connection is three PRs ago. The build gate
still said otherwise:
src/room/*.csat behind--enable-websocket, theRoomclass was guarded, and each of the six
HttpServerroom methods carried an#elsearm that threw "Rooms require the extension built with WebSocketsupport". A build configured with
--disable-websocketnow compiles the core,registers
Room, and delivers a publish from one thread to arecv()inanother.
The BC break
RoomDeliveryExceptionmoves tostubs/RoomExceptions.phpand extendsHttpServerExceptioninstead ofWebSocketException— rooms are served whereWebSocketExceptiondoes not exist. A handler that caughtWebSocketExceptionaround
Room::send()/HttpServer::send()no longer catches it.CHANGELOG.mdcarries it under Unreleased, and test 059, which asserted the old chain, asserts
the new one.
Nothing else about the exception changed: same
delivered/pendingcounts,same message, same distinctness from
WebSocketBackpressureException.Two defects only this configuration could show
Both were found by building it, and both are fixed here:
getRuntimeStats()reported no room counters. The whole block sat behindthe WebSocket guard, though the counters belong to the core. Every build
reports them now; the
ws_*key names stay, because php-claw and the testsread them.
room_hub_thread_sweep()sat behind the guard too. A thread thatsubscribed to a room has no other detach, so without WebSocket it never
detached: a leaked mailbox and a live libuv handle at request shutdown
(
uv_loop_close failed (EBUSY)).The gate
A step on the debug leg builds
--disable-websocketin a copy of the tree andruns
tests/phpt/room. Review found the first version of it broken in threeways, all fixed:
-d extension=<abs path>while the runner's scan dir alreadyloads the same module by name from the WebSocket build. PHP keeps the first
load and discards the second, so the probe saw
TrueAsync\WebSocketand failedon every run. Reproduced locally with a scan-dir ini, and
-d extension_dirfixes it — measured both ways.
--enable-http-server, so it built an HTTP/1-only,TLS-less extension: it answered "do the room sources link" rather than "does
the product build". It now takes the same flags as the real build, minus
WebSocket.
run-testsexits 0 wheneverything skips. The ten connectionless room tests move from
tests/phpt/websocket/totests/phpt/room/, the leak lane follows them, andthe step now asserts that at least ten tests actually passed.
Measured
tests/phpt382 passed, 0 failed, 1 warned (an XFAIL sectionon a passing test, pre-existing).
tests/valgrind-rooms.sh10 tests, 0 leaked.--disable-websocketbuild: no warnings of ours,nm -ushows no undefinedsymbol of ours, the extension loads,
TrueAsync\RoomandTrueAsync\RoomDeliveryExceptionexist withWebSocketabsent, and all 11tests in
tests/phpt/roompass.Corrected on the way
The "Rooms are not available" message told the caller to use
addWebSocketHandler(), which never allocated the hub in either build; the samefalse claim was in
stubs/HttpServer.php. Also fixed: a runtime message about"the WebSocket topic hub", the
Roomstub describing a subscriber as a WebSocketconnection,
docs/PLAN_RELIABLE_ROOM_PUBLISH.mdon the exception's parent, andthree file comments that asserted the opposite of what the code now does.