Opened on 01/19/2017 at 06:07:25 PM
Closed on 07/28/2017 at 06:49:17 AM
#4826 closed defect (fixed)
Unit tests have race conditions arising from asynchrony in I/O and web request
Reported by: | eric@adblockplus.org | Assignee: | eric |
---|---|---|---|
Priority: | Unknown | Milestone: | |
Module: | Libadblockplus | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | #4938 | Platform: | Unknown / Cross platform |
Ready: | yes | Confidential: | no |
Tester: | Unknown | Verified working: | no |
Review URL(s): |
Description
Environment
The unit tests for libadblockplus are full of calls that sleep the thread of the test body in order to wait for asynchronous calls in the engine to complete. These calls all represent race conditions in the tests. All such sleeping arises either from file system I/O or from web requests, both of which use separate threads running asynchronously.
Using the development branch fixing #3593, certain of the unit tests crash when run under the debugger because of these race conditions.
Expected behaviour
The tests should not crash, at minimum. The race conditions should be eliminated.
Attachments (0)
Change History (5)
comment:1 Changed on 01/19/2017 at 06:11:59 PM by eric@adblockplus.org
- Review URL(s) modified (diff)
comment:2 Changed on 02/27/2017 at 10:22:43 AM by sergz
- Blocked By 4938 added
comment:3 Changed on 02/27/2017 at 10:41:09 AM by sergz
- Blocked By 4938 removed
- Blocking 4938 added
comment:4 Changed on 05/26/2017 at 01:54:29 PM by sergz
- Blocked By 3593 removed
comment:5 Changed on 07/28/2017 at 06:49:17 AM by sergz
- Resolution set to fixed
- Status changed from new to closed
No unit tests sleeping in order to wait for asynchronous operations related to file system and web requests left. Even more, they are not using threads, so currently there are no race conditions and they don't crash.
However, sleeps are still used in order to test the default implementation of timer but it seems it would be better to address it in another issue.
The solution in https://codereview.adblockplus.org/29372702/ uses a latch that arrives in a callback and is waited on in the test body. This changes eliminates sleepy race conditions in all but two unit tests. The remaining two cannot use this technique because no callback interface is available to the tests.