Skip to content

fix: ensure Topic#publish1 returns accurate result during concurrent closure#3665

Open
guptapratykshh wants to merge 4 commits intotypelevel:mainfrom
guptapratykshh:fix/issue-3644-topic-publish-race
Open

fix: ensure Topic#publish1 returns accurate result during concurrent closure#3665
guptapratykshh wants to merge 4 commits intotypelevel:mainfrom
guptapratykshh:fix/issue-3644-topic-publish-race

Conversation

@guptapratykshh
Copy link
Contributor

this pr ensures Topic#publish1 accurately returns Left(Closed) when delivery fails due to concurrent topic closure by checking individual subscriber channel results and also guarantees that Right(()) response from publish1 consistently signifies successful delivery to all currently registered subscribers.

Resolves #3644

@guptapratykshh
Copy link
Contributor Author

@mpilquist can you have a look at this PR , Thanks.

@mpilquist
Copy link
Member

Need to rebase this PR now that other PRs have been merged. Also, need to remove the .fail from this test case: #3645

@guptapratykshh guptapratykshh force-pushed the fix/issue-3644-topic-publish-race branch 2 times, most recently from da8bc03 to bd2981e Compare January 26, 2026 05:15
@guptapratykshh guptapratykshh force-pushed the fix/issue-3644-topic-publish-race branch from bd2981e to a30644f Compare January 26, 2026 05:17
@mpilquist
Copy link
Member

@TomasMikula Mind reviewing this PR? Thanks!

@TomasMikula
Copy link
Contributor

So this PR guarantees that

  • if publish1 returns Right(()), then all subscribers receive the event

whereas the failing test also requires that

  • if publish1 returns Left(Closed), then no subscribers receive the event.

I admit the test is somewhat strict, and I'd consider the implemented behavior acceptable if clearly documented that return value Left(Closed) admits some subscribers receiving the event anyway.

}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor thing: I think constructing the effect in a right-associative way will lead to slightly faster execution for an IO-like F.

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.

The return value of Topic#publish1 is lying when there's a race condition with Topic closure

3 participants