Opened 8 months ago

Closed 5 months ago

#5198 closed defect (fixed)

Race condition in destroying of JsEngine

Reported by: sergz Assignee:
Priority: P2 Milestone:
Module: Libadblockplus Keywords:
Cc: fhd Blocked By: #4688
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29424663/

Description

The tests are crashing and it can happen with the regular use of it because of the following race condition.

When there is no any external reference to JsEngine it can happen that an async operation (e.g. web request) still keeps JsEngine while a callback is being executed, as the result the last strong reference to JsEngine is released from the worker thread, what means that JsEgnine is being destroyed from that worker thread, meanwhile it calls a destructor of the async component (web request/timer/FS) which calls join on the worker thread, but it's incorrect (dead-lock) to join a thread from the same thread and it throws an exception (BTW, from destructor).

Since we cannot really guarantee (it's very easy to make a mistake there) that the timer callback being executed does not obtain a strong reference to JsEngine we need some external mechanism waiting for finishing of all internal processes (IO, timers, JsEngine and FilterEngine are properly destroyed). It resembles #1285 because we could have a component consisting from several modules, such as FilterEngine and Updater, but in addition Prefs and Infrastructure (IO and timers), and this component could wait for finishing of its internals.

#5118

Change History (5)

comment:1 Changed 8 months ago by sergz

  • Blocking 5182 added

comment:2 Changed 8 months ago by sergz

  • Review URL(s) modified (diff)

Add temporary workaround.

comment:3 Changed 8 months ago by abpbot

A commit referencing this issue has landed:
Issue 5198 - workaround for race condition in tests

comment:4 Changed 7 months ago by sergz

  • Blocking 5182 removed

comment:5 Changed 5 months ago by sergz

  • Resolution set to fixed
  • Status changed from new to closed

Basically, now the issue can be considered as resolved because there is external Platform class which is responsible for the waiting, however without #5179 there is no waiting for file system operations and for web requests.

Note: See TracTickets for help on using tickets.