[HA Crash] iOS10 NSFetchRequest fails while servicing chrome.storage.set

[pavelz]

Environment

*iOS10*
ABB 1.5.1 Appstore release

How to reproduce

Unknown. Possibly an unfortunate timing of filter list update

Observed behaviour

Crash which may be completely sudden, as it is induced by usage of chrome.storage.local.set in the background script

MOST FREQUENT, all iOS10 versions
​https://rink.hockeyapp.net/manage/apps/310687/app_versions/27/crash_reasons/139881718

1/10th of the first one due to happening just on 10.0.2 which is being upgraded gradually to 10.1
​https://rink.hockeyapp.net/manage/apps/310687/app_versions/27/crash_reasons/139963337

Expected behaviour

No crash

[pavelz]

Follow up on the same issue in 1.5.1 release
https://issues.adblockplus.org/ticket/4435

[pavelz]

the calling path is roughly equal, but now it is a problem with the NSFetchRequest itself

Where the most frequest crash on all iOS10 versions
​https://rink.hockeyapp.net/manage/apps/310687/app_versions/27/crash_reasons/139881718
is fetching data in the merge method
​https://github.com/kitt-browser/kitt-core/blob/release/src/Persistence/ChromeStorage.swift#L78
​https://github.com/kitt-browser/kitt-core/blob/release/src/Persistence/BrowserStateCoreData.m#L183

The second most frequent, just on iOS10.0.2
​https://rink.hockeyapp.net/manage/apps/310687/app_versions/27/crash_reasons/139963337
is storing data in the same merge method
​https://github.com/kitt-browser/kitt-core/blob/release/src/Persistence/ChromeStorage.swift#L101
​https://github.com/kitt-browser/kitt-core/blob/release/src/Persistence/BrowserStateCoreData.m#L246

[pavelz]

The relevant crashing code in iOS core
​https://github.com/aosm/libmalloc/blob/master/src/nano_malloc.c

[pavelz]

Containment strategies:

1. (Eyeo) sift the user feedback for some common objection
2. (Eyeo) test filterlist enabling/disabling AND BACKGROUND UPDATING (todo: how to induce that>?)
3. (Salsita) review the core code for all CoreData access, assuming and understanding the calling environment
4. (Salsita) instrument CoreData access points with main thread assertion, and run intensively
5. (Salsita) consider replacing CoreData with something simpler and more appropriate to the task (Realm.io or even plain NSUserDefaults)

[pavelz]

Containment update:

1. didn't turn out anything
2. no success yet
3. 4. reviewed and instrumented by @jand, no catch

We are proposing 5, and that in way of dropping usage of CoreData (which is SQLite beneath) for storage keys file:*. The storage content will be saved in file(s) with the actual name. This was the original usage pattern and the long megabytes of filter list content seem to be not the right pattern for SQLite.

Mind that this is all a speculative change, as we don't know what's the real problem for sure, until a reproduction scenario is found. But simplifying things is rarely a wrong thing to do.

[pavelz]

It was found out that chrome.storage.set with key file:patterns.ini is called TWICE per a filter list update, with a slight delay of 1-2 seconds. This is reproducible on desktop Chrome. So the latest speculative crash scenario is an autoupdate which fires right in the middle of loading a heavy webpage. Then two threads are competing intensively and concurrently over allocation in one app's memory pool: app main thread on behalf of CoreData and webthread on behalf of WebKit. It would explain why the bug is so hard to reproduce. Not even an UI button to force subscription update (a la options page) would simulate - a web page needs to be loading at the same moment.

So yet another possible mitigation arises: defer CoreData storing if a page load is underway at the moment.

Preventing ABP extension from storing those 5-10MB twice would be great as well, of course.

[fhd]

Yup, I can verify that chrome.storage.local.set is being called twice when a filter list update is forced in ABP for Chrome on desktop.

The two write operations are triggered at about the same time, by a subscription.updated and subscription.downloadStatus event respectively (see ​https://hg.adblockplus.org/adblockpluscore/file/tip/lib/filterListener.js). The first write happens immediately, while the second write is being queued, and happens after the first one finishes.

But there should be just one write in the first place. I'll investigate this, but in the mean time, here's an idea for a workaround: You could block for a little after each write, maybe that could avoid crashing when processing queued writes.