Opened 3 years ago

Closed 2 years ago

#4826 closed defect (fixed)

Unit tests have race conditions arising from asynchrony in I/O and web request

Reported by: eric@… 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.

Change History (5)

comment:1 Changed 3 years ago by eric@…

  • 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 3 years ago by sergz

  • Blocked By 4938 added

comment:3 Changed 3 years ago by sergz

  • Blocked By 4938 removed
  • Blocking 4938 added

comment:4 Changed 2 years ago by sergz

  • Blocked By 3593 removed

comment:5 Changed 2 years ago 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.

Note: See TracTickets for help on using tickets.