Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Timeout.cancel for waitUntilNotified #1391

Merged
merged 2 commits into from
Dec 14, 2023

Conversation

swankjesse
Copy link
Collaborator

This triggers a 'spurious wakeup' in the case where a timeout is canceled. This is the best we can do without a discrete mechanism to distinguish between a notify() and a timeout.

This triggers a 'spurious wakeup' in the case where a timeout
is canceled. This is the best we can do without a discrete
mechanism to distinguish between a notify() and a timeout.
@swankjesse
Copy link
Collaborator Author

I intend to implement Timeout.cancel() for awaitSignal(), intersectWith(), ForwardingTimeout, and AsyncTimeout in follow-up changes.

@swankjesse swankjesse requested a review from yschimke December 14, 2023 03:42
* A sentinel that is updated to a new object on each call to [cancel]. Sample this property
* before and after an operation to test if the timeout was canceled during the operation.
*/
@Volatile private var cancelMark: Any? = null
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also considered doing cancelCount: Int but I think I like this better. It doesn’t have an overflow problem!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would have worked. Concurrent read, add, writes would still have changed the value. Overflowing still results in a new value. The only problem would have been if you called cancel 2^32 times during an operation it wouldn't be detected (but that's probably okay because it's impossible).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That’s exactly the case I was considering. It’s practically impossible but ugly

Copy link
Member

@oldergod oldergod left a comment

Choose a reason for hiding this comment

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

ΛΓΤΜ

@swankjesse swankjesse merged commit 84823bd into master Dec 14, 2023
11 checks passed
@swankjesse swankjesse deleted the jwilson.1213.starting_cancel branch December 14, 2023 14:56
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.

5 participants