Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-93647

CancellationToken::isCancelable is inconsistent with documentation and incomplete

    • Type: Icon: Task Task
    • Resolution: Unresolved
    • Priority: Icon: Major - P3 Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • Server Programmability

      CancellationToken provides an isCancelable method, which claims by its documentation to return true if the Token's associated Source was destroyed without being canceled. That suggests to me that a Token that has already been canceled would return true for isCancelable since the Source either is still in scope, or was destroyed having been canceled.

      This API would be particularly useful if it did what it claimed because we could condition chaining continuations on onCancel on the Token being in a state where it's already been canceled, in which case the future immediately fires, or it could be canceled down the line. Preventing the cancellation chaining in all other cases means we can avoid cluttering the execution resources that will be used to run the continuations with useless no-ops that result from source dismissal.

      Instead, the API returns true only if the token could be canceled, and returns false if it has already been canceled. This means that in order to save execution resources in the way suggested above, we would have to use token.isCanceled() || token.isCancelable() as the condition, but even that is insufficient because the checks must be performed atomically.

      At the very least, this ticket should ensure the documentation for isCancelable reflects its implementation after having made any changes. I would also like for some API to expose the logical "or" of the isCancelable and isCanceled conditions so that we can save execution resources on dismissals. If we can do so in a way that preserves the correctness of existing uses of isCancelable, I'd prefer to just update the method to actually do what the current documentation claims it does.

            Assignee:
            Unassigned Unassigned
            Reporter:
            james.bronsted@mongodb.com James Bronsted
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated: