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):

https://codereview.adblockplus.org/29372702/

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)

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.

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.

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from eric.
 
Note: See TracTickets for help on using tickets.