Closed Bug 1173970 Opened 6 years ago Closed 6 years ago

[Linter: Wakelock] Audit wakelock usage in GeckoApp


(Firefox for Android Graveyard :: General, defect)

Not set


(Not tracked)



(Reporter: mcomella, Assigned: esawin)




(1 file)

via android lint:

 ../../src/main/java/org/mozilla/gecko/ The release() call is not always reached

2400               mWakeLocks.put(topic, wl);
2401             }
2402         } else if (!state.equals("locked-foreground") && wl != null) {
2403             wl.release();
2404             mWakeLocks.remove(topic);
2405         }

Priority: 9 / 10
Category: Performance
Severity: Warning
Explanation: Incorrect WakeLock usage.
Failing to release a wakelock properly can keep the Android device in a high power mode, which reduces battery life. There are several causes of this, such as releasing the wake lock in onDestroy() instead of in onPause(), failing to call release() in all possible code paths after an acquire(), and so on.

NOTE: If you are using the lock just to keep the screen on, you should strongly consider using FLAG_KEEP_SCREEN_ON instead. This window flag will be correctly managed by the platform as the user moves between applications and doesn't require a special permission. See 


Ensure the usage is correct (it might be - the wakelocks are stored in an array for an extended duration which could be screwing the linter up) and if so, suppress the warning with "@SuppressLint("Wakelock")".
Eugen, can you audit this (or pass it on) to make sure we're correctly taking and releasing wakelocks?

If so, add the @SuppressLint("Wakelock") annotation to the function.
Flags: needinfo?(esawin)
The condition the linter complains about is correct (although it's clear why the linter doesn't get the context of it) and there doesn't seem to be an issue with us keeping the screen wake lock when being in the background.

We could add a check in onPause to verify that we are not holding any wake locks, but that would introduce some complexity and potentially even race conditions (what to do onResume if we did hold some?).

I think this should be safe to suppress.
Flags: needinfo?(esawin)
Assignee: nobody → esawin
Comment on attachment 8626724 [details] [diff] [review]

Review of attachment 8626724 [details] [diff] [review]:

Sounds good to me!
Attachment #8626724 - Flags: review?(michael.l.comella) → review+
Thanks for taking care of this, Eugen!
Since Pulsebot seems dead, here is the push:
Closed: 6 years ago
Resolution: --- → FIXED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.