Opened on 04/28/2017 at 08:56:43 AM

Closed on 07/31/2017 at 03:14:15 PM

#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

Attachments (0)

Change History (5)

comment:1 Changed on 04/28/2017 at 10:45:06 AM by sergz

  • Blocking 5182 added

comment:2 Changed on 04/28/2017 at 02:31:17 PM by sergz

  • Review URL(s) modified (diff)

Add temporary workaround.

comment:3 Changed on 04/28/2017 at 02:37:57 PM by abpbot

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

comment:4 Changed on 05/11/2017 at 10:09:55 AM by sergz

  • Blocking 5182 removed

comment:5 Changed on 07/31/2017 at 03:14:15 PM 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.

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 (none).
 
Note: See TracTickets for help on using tickets.