Closed
Bug 744627
Opened 12 years ago
Closed 12 years ago
TokenServerClient can call callback twice
Categories
(Cloud Services :: Firefox: Common, defect)
Cloud Services
Firefox: Common
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla14
People
(Reporter: gps, Assigned: gps)
References
Details
Attachments
(1 file)
4.17 KB,
patch
|
rnewman
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•12 years ago
|
||
My usual safety valve for this is to null out the callback after invoking it…
Assignee: nobody → gps
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•12 years ago
|
||
Let me know if you have any better suggestions.
Attachment #614220 -
Flags: review?(rnewman)
Assignee | ||
Comment 3•12 years ago
|
||
Ignore the import of utils.js.
Comment 4•12 years ago
|
||
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+
Assignee | ||
Comment 5•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e72c21511787
Target Milestone: --- → mozilla14
Comment 6•12 years ago
|
||
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.
Description
•