Opened on 06/09/2017 at 08:37:17 AM

Closed on 07/07/2017 at 12:54:27 PM

#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

Attachments (0)

Change History (3)

comment:1 Changed on 06/14/2017 at 03:10:05 PM 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 on 07/07/2017 at 12:53:46 PM by abpbot

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

comment:3 Changed on 07/07/2017 at 12:54:27 PM by asmirnov

  • Resolution set to fixed
  • Status changed from reviewing to closed

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.