Opened on 12/02/2016 at 06:45:05 PM
Closed on 07/31/2017 at 02:48:58 PM
#4692 closed change (fixed)
Don't keep strong references to JsEngine in worker threads
| Reported by: | sergz | Assignee: | eric@adblockplus.org |
|---|---|---|---|
| Priority: | P3 | Milestone: | |
| Module: | Libadblockplus | Keywords: | |
| Cc: | fhd, oleksandr, eric@… | Blocked By: | |
| Blocking: | #3595 | Platform: | Unknown / Cross platform |
| Ready: | no | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): |
https://codereview.adblockplus.org/29369365/ |
||
Description
Background
Having JsEngine forever alive already hurts. If we stay with working threads for async operations (what is pretty likely for a near future) we will need to do it anyway since the lifetime of the threads is scoped by the lifetime of corresponding instance of JsEngine, otherwise we will get a circular reference, JsEngine<>-Threads<>-JsEngine.
What to change
Use
- v8::Persistent instead of std::shared_ptr<JsValue>
- std::weak_ptr<JsEngine> instead of std::shared_ptr<JsEngine>
as types of members of worker threads.
Attachments (0)
Change History (6)
comment:1 Changed on 12/19/2016 at 10:50:51 PM by eric@adblockplus.org
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:2 Changed on 12/31/2016 at 10:54:10 PM by eric@adblockplus.org
- Owner set to eric@adblockplus.org
- Review URL(s) modified (diff)
comment:4 Changed on 03/24/2017 at 01:58:50 PM by abpbot
A commit referencing this issue has landed:
Issue 4692 - Dont' keep a strong reference from timer thread
comment:5 Changed on 05/26/2017 at 01:54:29 PM by sergz
- Blocking 3593 removed
comment:6 Changed on 07/31/2017 at 02:48:58 PM by sergz
- Resolution set to fixed
- Status changed from reviewing to closed
Currently the tasks for asynchronous operations don't keep permanently a strong reference to JsEngine.
Review https://codereview.adblockplus.org/29369365/ get the number of occurrences of shared_ptr<JsEngine> down to a single one that can't be removed until scheduling improves.
There was no need for weak_ptr; plain pointers were sufficient after careful examination of life cycle issues.