`CancellationToken.ToCancelationToken()` broken after `CancellationToken.TryReset()` about protopromise HOT 4 CLOSED

It's honestly a big pain to convert between these 2 cancelation token types efficiently. Since the CancellationTokenSource.TryReset() API was added, I'm thinking it may be worth it to retire Proto.Promises.CancelationToken and change all the APIs to accept System.Threading.CancellationToken instead. This could be done with minor breaking changes by making an implicit cast that just calls ToCancellationToken().

from protopromise.

After some more thought, I've realized that even not caching the source does not fix the issue if someone stores the token somewhere and tries to reuse it after TryReset has been called. The only real fix will be to retire Proto.Promises.CancelationToken and adopt System.Threading.CancellationToken in its place.

[Edit] Actually, this can be fixed by wrapping the registration in another object and using its finalizer to notify when the callback is dropped. (See Stephen Toub's solution) It's not perfect as it won't be observable until the GC runs and finalizers are invoked, but it at least solves the issue.

As a bonus, we can also use that to store a collection of WeakReferences to those wrappers for the cache instead of doing the hacky way of extracting the source from the token.

from protopromise.

After attempting to fix this using the finalizer approach, I realized there is still another case I can't catch: It is possible the source has TryReset() then Cancel() called, then the source is collected before our finalizer is called, making it impossible to detect that the source was canceled after our callback was unregistered.

So I guess the only real fix is to retire Proto.Promises.CancelationToken and adopt System.Threading.CancellationToken after all...

from protopromise.

I think I may have actually figured out how to fix this without abandoning Proto.Promises.CancelationToken. I'm working on #89 for it.

from protopromise.