gpslogger icon indicating copy to clipboard operation
gpslogger copied to clipboard

Stops logging when Android reaches low memory

Open TheLastProject opened this issue 2 years ago • 8 comments

Sometimes, my device gets low on memory, and it causes GPSLogger to stop logging until there is memory again.

This behaviour by itself is completely reasonable, as otherwise Android may OOM-kill GPSLogger or other apps.

The problem with this is that there I never realize this until I get home after work to come home to a completely dark house because Home Assistant didn't know I arrived home and turned the lights on. It's always an easy fix, just tap the notification to cause Android to give the app more memory as it gets foregrounded, but I'd like a change in behaviour.

I don't know the exact reasoning behind the low memory behaviour, but I'd like to suggest one of the following changes:

  1. An option to continue logging on low memory (I do not know about the possible side-effects of this)
  2. A notification on a separate notification channel to be created on low memory, so GPSLogger alerts me and I can tap the notification to give it focus and for Android to prioritize it again. The reason to make this a separate channel is so I can mute the regular background notification

I'm willing to try to help code whatever change you consider to be reasonable, but I'd prefer to first see what you think.

image

In the above screenshot, I opened the app around 18:00. As you can see, the logging immediately started then again.

TheLastProject avatar Feb 14 '22 17:02 TheLastProject

Hello hello, OK an interesting issue. So from my understanding, onLowMemory, where that message originates,

While the exact point at which this will be called is not defined, generally it will happen when all background process have been killed. That is, before reaching the point of killing processes hosting service and foreground UI that we would like to avoid killing.

It sounds like GPSLogger's service is about to be killed at this point, so I don't think it's possible to continue logging, unless you know there's a way to force it.

Creating a new notification might be possible, on a new channel yes, but wouldn't that get removed as well when the app gets killed?

I've got another suggestion, maybe it's useful - what about sending a broadcast from GPSLogger indicating there's low memory, that could be useful if you have an automation app, so it could listen and then act on that broadcast. That way it isn't subject to GPSLogger being killed off?

mendhak avatar Feb 15 '22 22:02 mendhak

It sounds like GPSLogger's service is about to be killed at this point, so I don't think it's possible to continue logging, unless you know there's a way to force it.

I don't think it sounds like Android wants to kill GPSLogger. The way I interpret is, specially with the text "that we would like to avoid killing", is that Android asks apps to limit their memory usage so it can avoid having to kill anything. It sounds possible to me to keep logging or at least resume it after some minutes (possibly by setting a timer).

Creating a new notification might be possible, on a new channel yes, but wouldn't that get removed as well when the app gets killed?

Well, every time this happens to me, when I check my phone the GPSLogger app is there with the notification text "GPSLogger is still logging" so I think it might not get killed even though this LowMemory message gets triggered several times over the day.

I've got another suggestion, maybe it's useful - what about sending a broadcast from GPSLogger indicating there's low memory, that could be useful if you have an automation app, so it could listen and then act on that broadcast. That way it isn't subject to GPSLogger being killed off?

While I understand your suggestion, wouldn't this require some app to retrieve the broadcast (and thus, start up if not already running) during a low memory situation? Wouldn't that just make the low memory situation worse and increase the risk of any app (including GPSLogger) getting killed?


I think above everything I don't understand why it stops logging location on low memory in the first place, as I see nothing in the onLowMemory handler that would cause this (or well, anything at all). Do you understand why it stops logging on low memory?

TheLastProject avatar Feb 16 '22 18:02 TheLastProject

I can't say why it's stopping logging on low memory, I want to guess is that the background service is first getting warnings then being killed off, but this isn't an area I'm knowledgeable in. The vague onLowMemory documentation isn't helping either. Something is happening to at the very least freeze the service, and inform it multiple times thereafter of memory pressure.

I'd be able to troubleshoot it and maybe zoom in on a cause if I could emulate low memory situation, but it doesn't seem possible. Plenty of old StackOverflow threads exist, but none of those commands work for me. Perhaps it used to work in older versions of the emulator or I'm doing something incorrect.

It is worth nothing, the GPSLoggingService already uses an AlarmManager to set the next time to do the logging, and that's supposed to be independent of whether the application is running or not. So I'd think that the alarm would kick in even if the application got killed. This is to address the keep-logging bit, what I mean is, since the app is still alive, it should just continue. In theory.

Now, regarding putting a timer/alarm in onLowMemory to resume after a few minutes. I think I understand it, it's like saying, just wait a few minutes for things to calm down, then start logging again. I suppose it could be added in, it could just copy the same code as the existing alarm but hardcode the time to like 5 minutes.

        Intent i = new Intent(this, GpsLoggingService.class);
        i.putExtra(IntentConstants.GET_NEXT_POINT, true);
        PendingIntent pi = PendingIntent.getService(this, 0, i, 0);
        nextPointAlarmManager.cancel(pi);
        nextPointAlarmManager.setExactAndAllowWhileIdle(AlarmManager.ELAPSED_REALTIME_WAKEUP, SystemClock.elapsedRealtime() + 30000, pi);

But I am hesitant to add this without testing its effects, though maybe if you are frequently running into this scenario you could try it out.

mendhak avatar Feb 17 '22 22:02 mendhak

It is worth nothing, the GPSLoggingService already uses an AlarmManager to set the next time to do the logging, and that's supposed to be independent of whether the application is running or not. So I'd think that the alarm would kick in even if the application got killed. This is to address the keep-logging bit, what I mean is, since the app is still alive, it should just continue. In theory.

In theory that should work, yeah. I wonder if I end up getting very unlucky and the service gets OOM-killed for me right after cancelling and before queuing the next alarm:

https://github.com/mendhak/gpslogger/blob/c8e4fbd64ca83b35cb31833742792323b5ea8fd5/gpslogger/src/main/java/com/mendhak/gpslogger/GpsLoggingService.java#L1001-L1010

That's just a random guess though... However, looking at https://developer.android.com/reference/android/app/AlarmManager#set(int,%20long,%20android.app.PendingIntent) it does seem like the .cancel() may be unnecessary as "If there is already an alarm scheduled for the same IntentSender, that previous alarm will first be canceled."

(Or maybe there are other places in the code with more delay between receiving the alarm and scheduling a new one)

Now, regarding putting a timer/alarm in onLowMemory to resume after a few minutes. I think I understand it, it's like saying, just wait a few minutes for things to calm down, then start logging again. I suppose it could be added in, it could just copy the same code as the existing alarm but hardcode the time to like 5 minutes.

This might actually help, maybe in another place the service gets killed before it plans the next alarm. This way it could help to ensure an alarm is always set, even if GPSLogger gets unexpectedly killed in the middle of work. Though it does feel like a hack around other places of the codebase not setting a new alarm as soon as possible.

But I am hesitant to add this without testing its effects, though maybe if you are frequently running into this scenario you could try it out.

It is quite sporadic, like, somewhere between once a week or once a month. So I wouldn't say "frequently" but just frequently enough to notice something isn't going right.

TheLastProject avatar Feb 18 '22 15:02 TheLastProject

Since this last post I did try emulating low memory situations but nothing ever came from them. I tried infinitely appending to lists in memory, I also found some applications that would emulate low memory situations, and some adb commands to simulate low memory callbacks (which don't seem to exist any more). But none of them hit the on low memory code, so I wasn't able to test.

Have you hit the OOM situation since the last post? I'm wondering if I should add the code anyway to a test APK but it feels very uncertain.

    @Override
    public void onLowMemory() {
        LOG.error("Android is low on memory!");
        Intent i = new Intent(this, GpsLoggingService.class);
        i.putExtra(IntentConstants.GET_NEXT_POINT, true);
        PendingIntent pi = PendingIntent.getService(this, 0, i, 0);
        nextPointAlarmManager.cancel(pi);
        nextPointAlarmManager.set(AlarmManager.ELAPSED_REALTIME_WAKEUP, SystemClock.elapsedRealtime() + 300000, pi);
        super.onLowMemory();
    }

mendhak avatar May 15 '22 17:05 mendhak

It still happens to me (often enough that I ended up just disabling the app for now).

I will happily run a test APK for you and report back (can also build myself if you prefer, do I just replace onLowMemory on the latest git commit with that and that's it?)

TheLastProject avatar May 15 '22 18:05 TheLastProject

Oh yes if you can build it that would be very helpful. But if building it becomes bothersome just let me know, and I'll do an APK.

Yes just replace the onLowMemory, here, with this:

    @Override
    public void onLowMemory() {
        LOG.error("Android is low on memory!");
        Intent i = new Intent(this, GpsLoggingService.class);
        i.putExtra(IntentConstants.GET_NEXT_POINT, true);
        PendingIntent pi = PendingIntent.getService(this, 0, i, 0);
        nextPointAlarmManager.cancel(pi);
        nextPointAlarmManager.set(AlarmManager.ELAPSED_REALTIME_WAKEUP, SystemClock.elapsedRealtime() + 300000, pi);
        super.onLowMemory();
    }

The idea is to set an alarm for 5 minutes later (300000 ms) and it should then resume logging.

mendhak avatar May 15 '22 18:05 mendhak

Sorry for the delay, I just built my own APK from commit 603f109b73381fe42b1d4fcb5cef9944bb44b637 with the above onLowMemory change and will keep it running in the background to see if it makes a difference.

TheLastProject avatar May 29 '22 21:05 TheLastProject

Has this been working out for you?

mendhak avatar Jan 06 '23 21:01 mendhak

Oops, sorry, completely forgot to communicate this back. While I haven't ran the app in several months after I reinstalled my device to debug a battery drain issue (which turned out to be Home Assistant's fault), I did run it for several months after my last comment and I don't recall it ever stopping to log during those months. So, as far as I can tell, this did fix it :)

TheLastProject avatar Jan 06 '23 23:01 TheLastProject

No problem I'll just include this code block in the next milestone.

mendhak avatar Jan 07 '23 09:01 mendhak

v126 is now in releases and fdroid.

mendhak avatar Feb 26 '23 18:02 mendhak