#6274 closed defect (fixed)

ANR on cancelling elemhide thread for load error

Reported by: asmirnov Assignee:
Priority: P2 Milestone:
Module: Libadblockplus-Android Keywords:
Cc: vickyyu Blocked By:
Blocking: Platform: Android
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29671561/

Description (last modified by asmirnov)

Environment

If load error happens (onReceivedError) when elemhide thread is running, the thread is locked while trying to cancel elemhide thread.

How to reproduce

  1. Start navigate
  2. Close network connection (let's say disconnect Wi-Fi)
  3. See thread is locked in finish

Probably it's locked in https://github.com/adblockplus/libadblockplus-android/blob/master/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java#L1045

Observed behaviour

Thread is locked

Expected behaviour

Thread is locked but then unlocked (when elemhide thread finishes).

Attachments (3)

WechatIMG172.png (16.5 KB) - added by asmirnov 23 months ago.
g2jOckli.png (202.9 KB) - added by asmirnov 23 months ago.
WechatIMG167.png (121.7 KB) - added by asmirnov 23 months ago.

Download all attachments as: .zip

Change History (12)

Changed 23 months ago by asmirnov

comment:1 Changed 23 months ago by asmirnov

  • Description modified (diff)

comment:2 Changed 23 months ago by asmirnov

  • Cc vickyyu added

Changed 23 months ago by asmirnov

Changed 23 months ago by asmirnov

comment:4 Changed 23 months ago by asmirnov

This is dealock happened:

elemhide thread: finish() -> in onFinished() enter [finishedRunnableLockObject] -> in elemHideThreadFinishedRunnable.run() enter [elemHideThreadLockObject] [LOCKED]
main     thread: stopAbpLoading() -> enter [elemHideThreadLockObject] -> in elemhideThread.cancel() -> finish() -> in onFinished() enter [finishedRunnableLockObject] [LOCKED]

So main thread locked elemHideThreadLockObject and it's locking bg thread. Meanwhile bg thread locked finishedRunnableLockObject and it's locking main thread.

The logical reason is that finishing (but not yet finished so thread ref is not null as it's set to null at the very end of finishing) thread is being cancelled.

There are multiple ways of fighting with it (see "Java concurrent in Practice") and i believe the right one is avoid cancelling thread that is finishing. So we should mark it's finishing and skip cancelling.

comment:5 Changed 23 months ago by asmirnov

BTW it was extremely rare case (thread both finishing and cancelling exactly at the same time).

comment:6 Changed 23 months ago by asmirnov

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

comment:7 Changed 23 months ago by asmirnov

  • Description modified (diff)

comment:9 Changed 23 months ago by asmirnov

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