Skip to content

Fix flaky EventListenerTest_Relay.cancelAsyncCall#9563

Closed
kamilkrzywanski wants to merge 1 commit into
lysine-dev:masterfrom
kamilkrzywanski:fix/event-listener-relay-cancel-race
Closed

Fix flaky EventListenerTest_Relay.cancelAsyncCall#9563
kamilkrzywanski wants to merge 1 commit into
lysine-dev:masterfrom
kamilkrzywanski:fix/event-listener-relay-cancel-race

Conversation

@kamilkrzywanski

Copy link
Copy Markdown

Summary

Fixes #9372.

EventListenerRelay accepts a single event then installs a successor via Call.addEventListener. Under Loom, cancelAsyncCall often cancelled while a connection event was mid-handoff, so Canceled hit a spent relay and never made it into the recorder:

expected Canceled but was [CallStart, ProxySelectStart, ..., ConnectStart]

A related failure (ConnectEnd without matching start) came from concurrent EventRecorder.logEvent order checks.

Changes

  • EventListenerRelay: one-shot accept is atomic; concurrent events on the active tail are queued and flushed after the successor is installed
  • EventRecorder: serialize start/end matching with enqueue
  • cancelAsyncCall: hold the call in an application interceptor after CallStart, cancel, then release (still an in-flight async cancel, without racing DNS/connect)
  • EventListenerRelayTest: deterministic regression that blocks inside successor addEventListener and cancels in that window

Regression proof

cancelDuringSuccessorHandoffIsRecorded fails without the relay change (only CallStart is recorded) and passes with it.

Test plan

  • ./gradlew :okhttp-testing-support:test --tests okhttp3.EventListenerRelayTest
  • ./gradlew :okhttp:jvmTest --tests okhttp3.EventListenerTest
  • spotlessCheck
  • Confirmed regression fails on pre-fix EventListenerRelay

Cancel racing connection events could hit a spent one-shot relay and
never record Canceled (especially under Loom). Queue concurrent events
on the active tail until the successor is installed, serialize
EventRecorder start/end matching, and hold cancelAsyncCall in an
application interceptor until after CallStart.

Closes lysine-dev#9372
@JakeWharton

Copy link
Copy Markdown
Collaborator

Hi! Thanks for the PR, but we are imminently about to ban LLM descriptions and comments, as well as LLM-authored code. Your PR description is pretty clearly LLM authored and that makes me think the code is too (I didn't review the diff).

You are welcome to use whatever tools you want for investing problems, but we expect all code to be written by hand along with comments and descriptions. If you can meet these requirements, you're welcome to make a new PR with a fix.

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.

EventListenerTest_Relay.cancelAsyncCall flaky

2 participants