Closed Bug 744627 Opened 12 years ago Closed 12 years ago

TokenServerClient can call callback twice

Categories

(Cloud Services :: Firefox: Common, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla14

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(1 file)

Currently, TokenServerClient may call the user-supplied callback function twice. It does this because _processTokenResponse() is wrapped inside a try..catch. processTokenResponse() calls the user-supplied callback function. So does catch { }. So, if the user-supplied callback throws, it can get invoked twice.

This is bad. Callbacks should only get called once. I will have a patch shortly.
My usual safety valve for this is to null out the callback after invoking it…
Assignee: nobody → gps
Status: NEW → ASSIGNED
Let me know if you have any better suggestions.
Attachment #614220 - Flags: review?(rnewman)
Ignore the import of utils.js.
Comment on attachment 614220 [details] [diff] [review]
Don't double call callbacks, v1

Review of attachment 614220 [details] [diff] [review]:
-----------------------------------------------------------------

::: services/common/tokenserverclient.js
@@ +164,5 @@
>          cb(new TokenServerClientNetworkError(error), null);
>          return;
>        }
>  
> +      let self = this;

I tend to prefer the use of bind instead of a closure here, but nbd.

@@ +167,5 @@
>  
> +      let self = this;
> +      function callCallback(error, result) {
> +        if (!cb) {
> +          self._log.info("Callback already called! Did it throw?");

These should be warn, not info.

@@ +191,3 @@
>          let error = new TokenServerClientError(ex);
>          error.response = this.response;
> +        callCallback(error, null);

Trailing blank line.
Attachment #614220 - Flags: review?(rnewman) → review+
https://hg.mozilla.org/mozilla-central/rev/e72c21511787
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: