Skip to content

objects/uts: pin the sync-wait ordering with process_pending_events() - #519

Open
sacOO7 wants to merge 1 commit into
mainfrom
uts/objects-sync-wait-process-pending-events
Open

objects/uts: pin the sync-wait ordering with process_pending_events()#519
sacOO7 wants to merge 1 commit into
mainfrom
uts/objects-sync-wait-process-pending-events

Conversation

@sacOO7

@sacOO7 sacOO7 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Problem

The five sync-wait pseudocode blocks in uts/objects/unit/realtime_object.md (RTO23c1 ×3, RTO20e1 ×2) share a shape that is misleading for async SDKs: start an operation, ASSERT <future> IS NOT complete, then inject a channel-state change. The negative assert reads as immediately assertable, but on an SDK that dispatches get()/increment() asynchronously it is vacuous — true even if the operation hasn't started — and the subsequent state injection then races the operation's own dispatch. Depending on which wins, the SDK correctly takes either the pre-wait path (RTO23e/RTL33: 90001 or re-attach) or the parked-waiter path (92008) the block asserts. Both outcomes are spec-conformant; the pseudocode simply never pins which one the test exercises.

This is not theoretical: it caused a real CI failure in ably/ably-java#1228 (expected: <92008> but was: <90001>, reproduced locally 1-in-6 runs). ably-js's mocks process synchronously, so the reference implementation never exercises the gap — the pseudocode had no corrective there.

What changes

The corpus already defines the exact tool: process_pending_events() (uts/README.md §Conventions; writing-derived-tests.md §"prove a negative" mandates a yield before negative assertions). This PR deploys it explicitly in the five blocks, between the operation call and the negative assert, with pointer comments — the same per-site style realtime_client.md and channel_detach.md already use. The two FAILED blocks additionally explain the 90001-vs-92008 mechanism so future ports understand why the drain is load-bearing.

Representative before/after (RTO23c1-FAILED block):

# before
get_future = channel.object.get()
ASSERT get_future IS NOT complete
# inject ERROR -> FAILED ...

# after
get_future = channel.object.get()
# Drain queued async work so get() has parked its RTO23c sync waiter (and the negative
# assert below is meaningful) before the channel-state injection — see process_pending_events()
# in uts/README.md; async SDKs otherwise race the injection against get()'s own dispatch,
# observing the RTO23e/RTL33c pre-wait 90001 instead of the parked-waiter 92008.
process_pending_events()
ASSERT get_future IS NOT complete
# inject ERROR -> FAILED ...

Notes

The five sync-wait pseudocode blocks (RTO23c1 x3, RTO20e1 x2) start an
operation, assert its future IS NOT complete, then inject a channel-state
change. On async SDKs the negative assert is vacuous without a drain —
the operation's dispatch races the injection, so the test can observe the
RTO23e/RTL33 pre-wait outcome (90001 / re-attach) instead of the parked-
waiter 92008 the block asserts. This caused a real CI failure in
ably/ably-java#1228 (expected 92008, got 90001); ably-js's synchronous
mocks never exercise the gap, so the reference implementation gave no
corrective.

Deploy the corpus's existing process_pending_events() convention
(uts/README.md; writing-derived-tests.md "prove a negative") between the
operation call and the negative assert in all five blocks, with pointer
comments — the same per-site style realtime_client.md and
channel_detach.md already use. The two FAILED blocks additionally explain
the 90001-vs-92008 mechanism.

Not related to #518: that PR governs timer-driven work vs ADVANCE_TIME;
this is dispatch-queue ordering with no timers involved.

@maratal maratal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants