Skip to content

diam: deliver a close to a CloseNotify() caller that asks after it happened. - #270

Merged
fiorix merged 1 commit into
fiorix:mainfrom
shifeeq:fix-closenotify-after-close
Oct 8, 2026
Merged

fiorix merged 1 commit into
fiorix:mainfrom
shifeeq:fix-closenotify-after-close

Conversation

@shifeeq

@shifeeq shifeeq commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

closeNotify() creates the notify channel lazily, and notifyClientGone() only closed it if it already existed, recording nothing otherwise. So when a connection closed before anyone had called CloseNotify(), a later call created a fresh channel that nothing would ever close, and the caller blocked forever.

Record the close in notifyClientGone() whether or not anyone is listening, and have closeNotify() return an already-closed channel when the connection is already gone.

TestCloseNotifyAfterClose fails without this change and passes with it. TestCloseNotifyBeforeClose is the control.

…ppened

closeNotify() creates the notify channel lazily, and notifyClientGone() only closed it if it already existed, recording nothing otherwise. So when a connection closed before anyone had called CloseNotify(), a later call created a fresh channel that nothing would ever close, and the caller blocked forever.

Record the close in notifyClientGone() whether or not anyone is listening, and have closeNotify() return an already-closed channel when the connection is already gone.

TestCloseNotifyAfterClose fails without this change and passes with it. TestCloseNotifyBeforeClose is the control.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@shifeeq shifeeq changed the title diam: deliver a close to a CloseNotify() caller that asks after it ha… diam: deliver a close to a CloseNotify() caller that asks after it happened. Oct 6, 2026
fiorix added a commit that referenced this pull request Oct 8, 2026
Includes Shifeeq's original fix from #270 and deterministic lifecycle and concurrency regression tests.

Independently reviewed; full race/coverage tests, build, and vet pass.

Fixes #269.
@fiorix
fiorix merged commit 22bd182 into fiorix:main Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants