Wallpaper Preview mode spawns new animation threads

[schlimme-gegend]

The android wallpaper selection dialog seems to start the sketch in a new thread. To observe this, run this minimal live wallpaper:

int randomColor = color(0);
boolean isPreview;
boolean hasColor = false;

void setup(){
  isPreview = wallpaperPreview();
  fullScreen();
  colorMode(HSB);
}

void preview(){
  if(!hasColor){
    randomColor = color(random(255), 255, 255);
    hasColor = true;
  }
}

void draw(){
  if(isPreview){
    preview();
  }
  background(randomColor);
}

The preview shows a different color each time it is active, but the home screen is completely unaffected and stays black. Since isPreview is only set once in setup, this implies that the wallpaper selector restarts the sketch each time the preview is shown. Further inspection with the android studio debugger shows that the number of animation threads of the app increases with each time the wallpaper is selected. This makes it hard to communicate settings from preview to live mode and becomes a serious problem in cases where the preview uses relevant amounts of memory.

Thoughts on a solution

Restarting the sketch after the wallpaper is selected will not solve this, because this would reset all variables that where affected in preview mode. It may make sense to replace the main animation thread with the one spawned by the wallpaper selector, but I'm not sure about the possibility of this and what nasty side effects it may have.

[codeanticode]

@schlimme-gegend thanks for flagging this problem. It was tricky to implement the preview mode for live wallpapers, and it seems that the current solution has a serious issue. I will look into it asap!

[schlimme-gegend]

Great, and maybe the digging I did can take some work off your shoulders: It looks like PWallpaper.onCreateEngine() is a good point to start. It gets called each time the preview is shown, but only the first time 'setWallpaper' is tapped. After a wallpaper is selected, the PWallpaper instance already has an engine when onCreateEngine() is called the next time. Unfortunately

  public Engine onCreateEngine() {
    if(engine==null){
      engine = new WallpaperEngine();
    }
    return engine;
  }

did not have the desired effect but may be a step in the right direction.

I also looked at MainService.mActiveEngines. While in preview mode, this list has two entries, but only one otherwise. Index 0 seems to reliably refer to the sketch working as wallpaper.

Conclusions (aka wild guesses)

• onCreateEngine() creates new engines with associated animation threads that are not properly disposed after the preview is closed
• The android wallpaper service kind of keeps track of preview and live engines, this could be an opportunity to enable transfer of parameters between preview and live mode

[codeanticode]

@schlimme-gegend Many thanks for looking into this. Most of the code examples available online (like this tutorial or this old sample from Google) use the same technique of just returning a new wallpaper engine every time onCreateEngine() is created. I don't think the null test works because the object returned by onCreateEngine is stored somewhere else, probably in the Main.mActiveEngines list as you suggest.

Now that I think more about this, my guess is that the reference to the PApplet object might be keeping the preview instances of the wallpaper alive so they cannot be disposed by the GC... a way to test this could be to set engine.sketch to null in onDestroy(), currently PWallpaper.onDestroy() calls engine.onDestroy(), which in turns calls sketch.onDestroy(), but sketch contains to hold the instance to the main sketch. What do you think?

[schlimme-gegend]

To test your suggestion I made WallpaperEngine.sketch package private and modified PWallpaper.onDestroy() to this:

public void onDestroy() {
    super.onDestroy();
    if (engine != null){
      engine.sketch=null;
      engine.onDestroy();
    }
  }

When 'Select wallpaper' is tapped for the first time, the app crashes. The crash begins with a NoSuchArgumentExeption thrown by android.view.Display looking for a getRawWidth method, that escalates into an IllegalArgument exception (not sure what throws it) that then stops the app. Restarting the app from the 'has stopped' dialog then starts the wallpaper as soon as the desktop becomes visible. With the live instance running, opening the selector anew and setting wallpapers no longer crashes the app. I also never got more than three animation threads existing at the same time this way, which is an improvement.

Crashing while starting the live engine implies that onDestroy() gets called before the live instance is set up and that at least some parts of the preview sketch are required to do this. That it only happens the first time a wallpaper is selected implies, that this is a special occasion (I may be fixated on this, but I couldn't fail to observe that this is the only time where mActiveEngines.size() is below two).

[codeanticode]

This looks trickier than I thought :-)

To help you debug, I added a wallpaper sample project:

https://github.com/processing/processing-android/tree/super-fast-P2D/studio/apps/wallpaper

With this, you can use Android Studio to step through the code while running the sketch in a debug session and get a better sense of the init and de-init of a live wallpaper.

[schlimme-gegend]

Thanks. I augmented that with an empty activity that can be used as starting point in the run config and with this allows me to attach a debugger before starting the preview. I have a vague suspicion what causes the fatal `NoSuchArgumentExeption' in the setting above and will dig into that.

[schlimme-gegend]

So this is what I got so far:

• Nulling a thread does not stop it, and thus does not line it up for GC.
• The recommended way to stop threads is the use of the interrupt() method in combination with a flag that is tested in the run() method.
• Calling interrupt() on the waiting animation thread does neither throw an InterruptedException nor affect its built in interrupt flag. This conflicts with the article in the paragraph above.
• The stop() method is deprecated for a reason, in our case it instacrashes the app.
• The NoSuchArgumentExeption and the IllegalArgumentException mentioned above, likely were side effects of very slow debugging messing with the main Looper. They magically disappeared once I reduced the number of breakpoints and did no longer step into every single function on the way.

I'm running out of ideas over here after I tried the interrupt flag approach with no discernible effect. The unused animation threads linger on in the WAITING state and I could not find out how they got there. The wait() method of the the animation thread is never called in any part of the code I have access to. Even if it was, Thread.wait() is final and thus cannot be overridden to just dispose the thread (This is probably a good thing but stands in the way of figuring out where it is called).

I hope this helps, keep up the great work!

[codeanticode]

Thanks for all these insights! I'll see if I can find anything else? I wonder if you see similar unused threads in other live wallpapers, or only happens with wallpapers made with Processing?

[schlimme-gegend]

Good idea. The one live wallpaper I found on github that is less than 4 years old and implemented in java does not create dangling threads. Its wallpaperEngine uses a Handler for its animation thread and seems to perform at least some cleanup in the `onSurfaceDestroyed()' method.

The design seems to be heavily influence by this example from 2009. Note that my argument against old examples is mainly based in my aversion towards ever letting eclipse near me again, rather than technical considerations.

However, adding a Handler to PSurfaceNone and having removeCallbacks(thread) called in stopThread was not the silver bullet we're looking for.

[schlimme-gegend]

I'm pretty sure I solved it. Basically it is about substituting thread = null; with thread.interrupt(); in PSurfaceNone.stopThread() and then using the InterruptedException thrown inside checkPause() when a waiting thread is being interrupted to trigger the shutdown from inside the thread's run() method. Although this reeks of expection handling, this seems to be the recommended way to stop threads that are occupied with a blocking method, such as wait(). The fix currently exists in a local branch that gitHub doesn't let me push due to insufficient permissions.

[codeanticode]

@schlimme-gegend that's great news, thanks so much for keeping at it! Why don't you create a pull request that I will then test and merge?

[codeanticode]

@schlimme-gegend I merged your changes, and I run a series of wallpaper tests. I don't see new animation threads spawn each time one opens the wallpaper in the preview mode, although I noticed a couple of new threads appearing under specific situations:

1) The first time I launch the wallpaper, I see the following threads:

2) If I open the wallpaper in the preview mode (while it is already running) one new GLThreads shows up:

3) But this new GLThread is removed after I leave the preview (the wallpaper is already selected, so it does not matter if I select it in preview, or cancel the selection):

4) The only situation I see a new thread being spawn is if I do the following: select a new wallpaper, then go back to the selector and pick the Processing wallpaper again. Then I see a new "Binder" and a "queued-work-looped" threads. It does not matter how many times I do this (changing the wallpaper, re-selecting the Processing wallpaper), the extra wallpapers are always these two, don't see new ones being spawned:

Would you say the original issue you reported is taken care of?

[schlimme-gegend]

I just looked into the memory use associated with each thread and found that the Binder threads use a negligible amount of RAM compared to the Animation threads. So the crippling bug has been resolved and the Binder threads can be considered a nuisance that should be taken care of sooner or later. They also look different from the initial problem, as they are kept in the RUNNING state. So I recommend to close this issue ('Preview mode spawns animation threads that linger on as memory devouring zombies') and open a new one along the lines of 'Preview mode litters the place with binders'.

Kind of related to this, there is the situation that processing currently provides no way of communicating between preview and active wallpaper. This means the preview can not readily be used to change the behavior of the wallpaper after it has been selected. A feature request seems to be the right place to discuss this.

[codeanticode]

Ok I will go ahead and close this issue. Note that restarting the wallpaper several times does not seem to keep adding Binder thread. Three is the maximum I have seen, no mater how many time I restart the wallpaper. I should look if this is something that happens with any wallpaper more generally, or only wallpapers made with Processing.

I don't know if there is the need to add extra functionality to save the state of the preview so the active wallpaper picks up that state. You can use existing functionality in the Android system. For example, you the preview instance could save a file to the app's internal cache space, which would be then loaded by the active instance. Wouldn't this be enough. I think there are other ways in Android to share data between apps.