Opened 2 years ago

Closed 2 years ago

#5309 closed defect (fixed)

Subscriptions update causes ANR

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):

https://codereview.adblockplus.org/29465639/

Description (last modified by sergz)

Environment

It was observed that usage of preloaded subscriptions causes ANR for few seconds.

While measuring method invocation durations it was found that it's AdblockEngine.updateSubscriptions() https://github.com/adblockplus/libadblockplus-android/blob/master/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java#L498 method to last from 10 ms to 1100 ms. Since it's invoked from main thread it blocks it and the app is frozen. We do use main thread to sync multiple subscriptions download attempts - clear callbacks queue and schedule new runnable invocation with 1000 ms delay:
https://github.com/adblockplus/libadblockplus-android/blob/master/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java#L289

So when 2 concurrent subscription download requests come the runnable is fired once only.

For some (unknown at the moment) reason method duration depends on multiple condition (subscription URL, device, etc) and i was able to reproduce it 2 of 5 times on one device (Galaxy Note 3) and never on ZTE device. The method goes to C++ https://github.com/adblockplus/libadblockplus-android/blob/master/libadblockplus-android/jni/JniSubscription.cpp#L76 and then to https://github.com/adblockplus/libadblockplus/blob/f5d7eb077080e0dd3c25a5bc31bf8b05340dc0fc/src/FilterEngine.cpp#L161.

So the original assumption is that JS invocation is blocked until some condition achieved. Probably it's because of filters list parsing. The parsing is know to take some time and for that time any call on JS engine will be stalled.

We have the following choices:
1) investigate the reason of JS blocks for sure (smart and difficult)
2) try to avoid JS blocks just scheduling updating subscription to timer thread (smart enough and much cheaper)
3) create new java/c++ thread in order not to block main thread and don't care about root cause (simple but will bring threads synch problems)

How to reproduce

  1. Make sure to delete all app cache and run multiple times

Observed behaviour

The app freezes for sometime, the cursor is not blinking

Expected behaviour

The app does not freeze, the cursor is blinking

Change History (3)

comment:1 Changed 2 years ago by sergz

  • Description modified (diff)
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

In addition I think forceUpdateRunnable should not update all subscriptions, it should update only the intercepted one because currently it schedules an update of a subscription which maybe not yet processed. I think there can be some race condition, for instance, it seems it can theoretically happen that the downloaded subscription can be overwritten by a preloaded one, though on practice it maybe difficult to reproduce.

comment:2 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5309 - Subscriptions update causes ANR

comment:3 Changed 2 years ago by asmirnov

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.