66

I have uploaded my application on google play but users have reported the following exception

java.lang.RuntimeException: WakeLock under-locked C2DM_LIB. This exception occurs when I try to release the WakeLock. Can anyone tell what could be the problem.

HitOdessit
  • 7,198
  • 4
  • 36
  • 59
Rookie
  • 8,660
  • 17
  • 58
  • 91

4 Answers4

196

You didn't post your code, so I don't know if you've already done what I will suggest here, but I also had that exception and all I added to fix it was a simple "if" to make sure the WakeLock is actually being held, before trying to release it.

All I added in my onPause was this "if" statement (before the "release()"):

if (mWakeLock.isHeld())
    mWakeLock.release();

and the exception was gone.

fasti
  • 2,379
  • 3
  • 16
  • 9
  • 10
    This solution seems a lot cleaner to me than the accepted one. – ottel142 Jun 13 '13 at 09:23
  • 1
    That's because it is -and- the right way to do it. This should have been the accepted answer. – CompEng88 Nov 25 '13 at 21:38
  • 2
    I have no .release() in my code (no mWakeLock what so ever) but I am still getting this error. The only stacktrace I see is: java.lang.RuntimeException: WakeLock under-locked GCM_LIB at [...]com.google.android.gcm.GCMBaseIntentService.onHandleIntent(GCMBaseIntentService.java:252) at android.app.IntentService$ServiceHandler.handleMessage(IntentService.java:65) – Ted Jan 31 '14 at 14:24
  • 4
    I agree that this should be the accepted answer but don't forget to put above code in a synchronized statement. I had rare cases where the wake lock was released by another thread between the calls to isHeld and release. – Emanuel Moecklin May 08 '14 at 15:56
  • 1
    do you have an example of the syncronized code? I think the safest is to use all 3 methods.. Catch Throwable, and isHeld, and Synchronized.. haha if an exception is raised at runtime, it is expensive.. save battery power by checking the isHeld first, it is more efficient, by a few microseconds, haha. – hamish Jul 12 '14 at 21:48
  • 1
    This solution failed in my case, I keep getting traces: http://crashes.to/s/9b51ed151b2 – josue.0 Apr 03 '18 at 07:02
  • This may look cleaner, but your app may still crash if the system releases your wakelock due to the time expiring in between the isHeld() and release() calls – morepork Sep 10 '18 at 18:47
  • doesn't work for me on the rare phone @Synchronized open fun stop() { if (wakeLock != null && wakeLock!!.isHeld()) { wakeLock!!.release() wakeLock = null } } – Crag Apr 11 '19 at 17:59
64

I have traced same exception in new GCM Library too. Actually old C2DM Android library have same error, same crash, and Google hasn't fixed it yet. As I can see by our statistics, about 0.1% of users experiencing this crash.

My investigations shows that problem is in incorrect releasing of network WakeLock in GCM library, when library tries to release WakeLock that holds nothing (internal lock counter becomes negative).

I was satisfied with simple solution - just catch this exception and do nothing, because we don't need to do any extra job then our wakelock hold nothing.

In order to do this you need to import GCM library sources in your project, rather than already compiled .jar file. You can find GCM library sources under "$Android_SDK_Home$/extras/google/gcm/gcm-client/src" folder (you need to download it first using Android SDK Manager).

Next open GCMBaseIntentService class, find line

sWakeLock.release();

and surround it with try-catch.

It should look like this:

    synchronized (LOCK) {
        // sanity check for null as this is a public method
        if (sWakeLock != null) {
            Log.v(TAG, "Releasing wakelock");
            try {
                sWakeLock.release();
            } catch (Throwable th) {
                // ignoring this exception, probably wakeLock was already released
            }
        } else {
            // should never happen during normal workflow
            Log.e(TAG, "Wakelock reference is null");
        }
    }

UPDATE: Alternativally, as suggested @fasti in his answer, you can use mWakeLock.isHeld() method to check if wakelock actually holding this lock.

Community
  • 1
  • 1
HitOdessit
  • 7,198
  • 4
  • 36
  • 59
  • 1
    Yes, I've implemented this solution in all our projects, it works perfectly (userbase more than 2M users) – HitOdessit Aug 27 '12 at 12:40
  • 3
    I went ahead and made this change in a fork of Google's repo and put it on Github: https://github.com/ajlyon/gcm – Avram Lyon Nov 20 '12 at 01:15
  • cool, can i just replace the GCM Library jar with this one ( https://github.com/ajlyon/gcm/blob/master/gcm-client/dist/gcm-src.jar ) and go on with that bug fixed? is it updated with the last release of GCM lib? – Mario Lenci Mar 13 '13 at 15:55
  • This is an old thread, so I am not sure if there any continual interest, but what i dont understand is that how can this happen. AFAI can see, this only possible if onHandle intent is called from outside of runIntentInService. Which should not happen ever correct? – user210504 Apr 03 '13 at 22:02
  • 4
    This solution is heavy handed and sweeps the problem under a rug. The one proposed by fasti below is the correct way of dealing with it. – twig Aug 11 '13 at 08:40
  • A yes, Catch Throwable, the sign of quality java – HaMMeReD Sep 23 '14 at 22:19
  • Catching and ignoring an exception is almost never a good practice. Especially so when, as in this case, there is a simple boolean check to handle the error case (see fasti's answer). – howettl Jan 30 '15 at 19:38
5

Although the isHeld() solution seems nicer, it can actually fail - because it is not atomic (i.e. not thread safe). If you have more than one thread that might release the lock then between the check (isHeld) and the call to relase another thread may release the lock... and then you fail.

By using try/catch you disguise the bug, but in a thread-safe way.

Shoham
  • 1,079
  • 1
  • 12
  • 17
  • 1
    Is there a good option to make a WakeLock release atomic in a reusable way? It should be an atomic operation. It literally has "Lock" in the name. – colintheshots Feb 12 '18 at 18:39
  • Are you sure? Looking at the PowerManager.java source code, it looks like these functions are synchronized. – Nick Nov 08 '18 at 15:42
1

I don't have this problem as long as I don't reinitialize the wake lock and call acquire on the new Object. You should only keep one instance of wakeLock (so make it a field variable). Then you know you're always releasing that one wakeLock.

So....

 if (mWakeLock == null) {
        PowerManager pm = (PowerManager) getSystemService(Context.POWER_SERVICE);
        mWakeLock = pm.newWakeLock(PowerManager.FULL_WAKE_LOCK | PowerManager.ACQUIRE_CAUSES_WAKEUP
                | PowerManager.ON_AFTER_RELEASE, "MyWakeLock");
    }

try{
        mWakeLock.release();//always release before acquiring for safety just in case
    }
    catch(Exception e){
        //probably already released
        Log.e(TAG, e.getMessage());
    }
    mWakeLock.acquire();
MobileMon
  • 8,341
  • 5
  • 56
  • 75