Closed Bug 920017 Opened 6 years ago Closed 6 years ago

Race condition in BackgroundFileSaver when setTarget is called multiple times in append mode

Categories

(Core :: Networking: File, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27
Tracking Status
firefox24 --- wontfix
firefox25 --- wontfix
firefox26 --- fixed
firefox27 --- fixed

People

(Reporter: Paolo, Assigned: Paolo)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

The Downloads API tests surfaced a race condition that may happen in
BackgroundFileSaver. In append mode, the first call to setTarget should set
the file to append to, and subsequent calls should allow it to be renamed.
However, if setTarget is called rapidly for two times, the worker thread has
no chance to notice the first call, and we end up appending to the wrong file.
Attached patch The patch (obsolete) — Splinter Review
Attachment #809164 - Flags: review?(mcmanus)
Attached patch Fixed patchSplinter Review
Multithreading is hard, this code is complex, and regression tests are good.

This patch fixes the completion condition, and a similar version passed try:

https://tbpl.mozilla.org/?tree=Try&rev=ce27bf80cec1
Attachment #809164 - Attachment is obsolete: true
Attachment #809164 - Flags: review?(mcmanus)
Attachment #809752 - Flags: review?(mcmanus)
Blocks: 906817
Blocks: 919100
Attachment #809752 - Flags: review?(mcmanus) → review+
https://hg.mozilla.org/mozilla-central/rev/c82c6b643f87
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Comment on attachment 809752 [details] [diff] [review]
Fixed patch

We might uplift this to Aurora to fix intermittent test failures.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 887425
User impact if declined: Virtually none, but fixes intermittent test failures.
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Low, has tests
String or IDL/UUID changes made by this patch: None
Attachment #809752 - Flags: approval-mozilla-aurora?
Attachment #809752 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.