1175355 - [Linter: HandlerLeak] Audit `new Handler()`s for resource leaks

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

HandlerLeak: Handler reference leaks
../../src/main/java/org/mozilla/mozstumbler/service/stumblerthread/scanners/cellscanner/CellScanner.java:75: This Handler class should be static or leaks might occur (new android.os.Handler(){})

  72                 new IntentFilter(Reporter.ACTION_NEW_BUNDLE));
  73 
  74         // This is to ensure the broadcast happens from the same thread the CellScanner start() is on
  75         mBroadcastScannedHandler = new Handler() {
  76             @Override
  77             public void handleMessage(Message msg) {

../../src/main/java/org/mozilla/gecko/background/preferences/PreferenceFragment.java:56: This Handler class should be static or leaks might occur (new android.os.Handler(){})

  53   // The issue is that the Handler references the Fragment; messages targeting
  54   // the Handler reference it; and if such messages are long lived, the Fragment
  55   // cannot be GCed. This is not an issue for us; our messages are short-lived.
  56   private final Handler mHandler = new Handler() {
  57     @Override
  58     public void handleMessage(Message msg) {

../../src/main/java/org/mozilla/search/SearchActivity.java:111: This Handler class should be static or leaks might occur (new android.content.AsyncQueryHandler(){})

 108         // Initialize the fragments with the selected search engine.
 109         searchEngineManager.getEngine(this);
 110 
 111         queryHandler = new AsyncQueryHandler(getContentResolver()) {};
 112 
 113         searchBar = (SearchBar) findViewById(R.id.search_bar);

../../src/main/java/org/mozilla/gecko/toolbar/ToolbarProgressView.java:71: This Handler class should be static or leaks might occur (new android.os.Handler(){})

  68         mBounds = new Rect(0,0,0,0);
  69         mTargetProgress = 0;
  70 
  71         mHandler = new Handler() {
  72             @Override
  73             public void handleMessage(Message msg) {

Priority: 4 / 10
Category: Performance
Severity: Warning
Explanation: Handler reference leaks.
Since this Handler is declared as an inner class, it may prevent the outer class from being garbage collected. If the Handler is using a Looper or MessageQueue for a thread other than the main thread, then there is no issue. If the Handler is using the Looper or MessageQueue of the main thread, you need to fix your Handler declaration, as follows: Declare the Handler as a static class; In the outer class, instantiate a WeakReference to the outer class and pass this object to your Handler when you instantiate the Handler; Make all references to members of the outer class using the WeakReference object. 

More info:

To suppress this error, use the issue id "HandlerLeak" as explained in the Suppressing Warnings and Errors section.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1175355 - Add WeakReferenceHandler. r=nalexander

Bug 1175355 - Add WeakReferenceHandler. r=nalexander

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1175355 - Add WeakReference into PreferenceFragment's Handler. r=nalexander

Bug 1175355 - Add WeakReference into PreferenceFragment's Handler. r=nalexander

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1175355 - Add AsyncQueryHandlerImpl in SearchActivity. r=nalexander

Bug 1175355 - Add AsyncQueryHandlerImpl in SearchActivity. r=nalexander

This avoids the "HandlerLeak" Android Lint warning.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1175355 - Add ToolbarProgressHandler into ToolbarProgressView. r=nalexander

Bug 1175355 - Add ToolbarProgressHandler into ToolbarProgressView. r=nalexander

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1175355 - Add BroadcastScanneredHandler to CellScanner. r=nalexander

Bug 1175355 - Add BroadcastScanneredHandler to CellScanner. r=nalexander

Comment 6

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8625987 [details]
MozReview Request: Bug 1175355 - Add AsyncQueryHandlerImpl in SearchActivity. r=nalexander

https://reviewboard.mozilla.org/r/11967/#review10575

::: mobile/android/search/java/org/mozilla/search/SearchActivity.java:74
(Diff revision 1)
> -    private AsyncQueryHandler queryHandler;
> +    private static AsyncQueryHandler queryHandler;

s/static//.

Comment 7

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8625989 [details]
MozReview Request: Bug 1175355 - Add BroadcastScanneredHandler to CellScanner. r=nalexander

https://reviewboard.mozilla.org/r/11971/#review10577

Ship It!

Comment 8

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8625985 [details]
MozReview Request: Bug 1175355 - Add WeakReferenceHandler. r=nalexander

https://reviewboard.mozilla.org/r/11963/#review10579

Ship It!

Comment 9

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8625986 [details]
MozReview Request: Bug 1175355 - Add WeakReference into PreferenceFragment's Handler. r=nalexander

https://reviewboard.mozilla.org/r/11965/#review10581

Ship It!

Comment 10

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8625987 [details]
MozReview Request: Bug 1175355 - Add AsyncQueryHandlerImpl in SearchActivity. r=nalexander

https://reviewboard.mozilla.org/r/11967/#review10583

Ship It!

Comment 11

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8625988 [details]
MozReview Request: Bug 1175355 - Add ToolbarProgressHandler into ToolbarProgressView. r=nalexander

https://reviewboard.mozilla.org/r/11969/#review10585

Ship It!

Comment 12

[Nick Alexander :nalexander [he/him]]

https://reviewboard.mozilla.org/r/11967/#review10587

Ship It!

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/a24f4a731038
https://hg.mozilla.org/integration/fx-team/rev/94af910d67fb
https://hg.mozilla.org/integration/fx-team/rev/42c4d687437d
https://hg.mozilla.org/integration/fx-team/rev/58334ada143a
https://hg.mozilla.org/integration/fx-team/rev/54453e0bbdc8

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a24f4a731038
https://hg.mozilla.org/mozilla-central/rev/94af910d67fb
https://hg.mozilla.org/mozilla-central/rev/42c4d687437d
https://hg.mozilla.org/mozilla-central/rev/58334ada143a
https://hg.mozilla.org/mozilla-central/rev/54453e0bbdc8