Opened on 01/23/2018 at 07:12:57 PM
Closed on 01/24/2018 at 01:29:37 PM
#6305 closed change (fixed)
Do not nullify storage on engine disposed
| Reported by: | asmirnov | Assignee: | |
|---|---|---|---|
| Priority: | P3 | Milestone: | |
| Module: | Libadblockplus-Android | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Platform: | Android | |
| Ready: | no | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description
Background
AdblockHelper acts as Facade for AdblockEngine and AdblockSettingStorage.
Typical lifecycle is:
1) init() (can be done once)
2) retain() (can be done multiple times by different clients)
3) use AdblockEngine instance returned with getEngine()
4) release() (can be done multiple times by different clients)
However when the last client calls release() AdblockProvider actually releases AdblockEngine and storage is released too:
https://github.com/adblockplus/libadblockplus-android/blob/master/libadblockplus-android-settings/src/org/adblockplus/libadblockplus/android/settings/AdblockHelper.java#L86
---
In complex lifecycle the client can call retain() after it's actually released and it will try to access already nullified storage:
https://github.com/adblockplus/libadblockplus-android/blob/master/libadblockplus-android-settings/src/org/adblockplus/libadblockplus/android/settings/AdblockHelper.java#L56
It should be consistent - both create/release in retain()/release() or keep single instance throughout all helper lifetime and do not release it on engine release.
What to change
Do not release storage on engine released or create it everytime on retain().
Attachments (0)
Change History (3)
comment:1 Changed on 01/23/2018 at 07:18:33 PM by asmirnov
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:2 Changed on 01/24/2018 at 01:29:13 PM by abpbot
comment:3 Changed on 01/24/2018 at 01:29:37 PM by asmirnov
- Resolution set to fixed
- Status changed from reviewing to closed
Some commits referencing this issue have landed: