Race condition in cleardata/tests/unit/test_downloads.js
Categories
(Toolkit :: Downloads API, enhancement, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox67 | --- | fixed |
People
(Reporter: jorendorff, Assigned: baku)
References
Details
Attachments
(1 file)
|
5.02 KB,
patch
|
johannh
:
review+
|
Details | Diff | Splinter Review |
Over in bug 1495072, I'm changing await to be faster. This makes a few tests fail, including this one:
I managed to trigger this by editing the test, with no changes to await: if I add await Promise.resolve(); at line 69, the test fails.
The test assumes that the second download, the one that is started on line 56, is still active by the time we call Services.clearData.deleteData. If the download has already completed, deleteData will remove both downloads from the list.
|
Reporter | |
Comment 1•5 years ago
|
||
baku, is there a way to make sure the download is still active when deleteData runs? If not, what can I do with this test?
|
|
Reporter | |
Comment 2•5 years ago
•
|
||
I noticed something else funny: the three methods of DownloadsCleaner all use a .then handler that returns undefined.
These methods always return promises that are already resolved, even though the downloads have not been removed yet.
So, even though ClearDataService._deleteInternal looks like it waits for the download-deleting process to be done, I think it actually isn't waiting for anything; the callback is called after 1 microtask tick regardless.
Anyway, fixing that doesn't fix the race condition in comment 0, so it's sort of beside the point.
|
Assignee | |
Comment 3•5 years ago
|
||
|
||
Updated•5 years ago
|
|
||
Comment 4•5 years ago
|
||
Comment on attachment 9039833 [details] [diff] [review] await.patch Review of attachment 9039833 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the delay :)
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4a8d5821feaa Fix test_downloads.js, r=johannh
|
||
Comment 6•5 years ago
|
||
| bugherder | ||
Description
•