-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Implemented the RegisterOnTermination feature. #1523
Implemented the RegisterOnTermination feature. #1523
Conversation
/// </summary> | ||
/// <param name="newValue">The new value</param> | ||
/// <returns>The current value</returns> | ||
public T GetAndSet(T newValue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation doesn't seem to be atomic. This operation is basically equivalent of return Interlocked.Exchange(ref atomicValue, newValue)
. Using [MethodImpl(MethodImplOptions.AggressiveInlining)]
also could be valuable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I struggled with this one, thanks for the hint, will update the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interlocked.Exchange
unfortunately only allows reference types in the generic version.
660bdfe
to
a136b77
Compare
Looks great! How does this integrate with the termination hooks for Akka.Remote? Or does this live as a separate system?
If the |
Sorry, but through my low understanding about Akka.Remote I can't give an appropriate answer. |
@Silv3rcircl3 ok, no worries Marc - I'll add that as something for me to investigate |
Implemented the RegisterOnTermination feature.
closes #1519
It also implements some of the tests for #60.