Skip to content

PApplet fragments retain Activity reference and prohibit GC #213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jsalerno opened this issue May 5, 2016 · 5 comments
Closed

PApplet fragments retain Activity reference and prohibit GC #213

jsalerno opened this issue May 5, 2016 · 5 comments
Assignees

Comments

@jsalerno
Copy link

jsalerno commented May 5, 2016

I am using the 3.02 android library.

I have 2 android.app.Activity subclasses, each of which embeds a different processing.core.PApplet fragment subclass. Those fragments extend my own subclass MyPApplet intermediate shown partially below, and MyTroublesomeProcessingFragment overrides the settings() method with a 3D setup:

public abstract class MyPApplet extends PApplet {
    @Override
    public void exit() {
        super.dispose();
    }
    ...
}

public class MyTroublesomeProcessingFragment extends MyPApplet {
    @Override
    public void settings() {
        size(width, height, PConstants.P3D);
        smooth();
    }
    ...
}

I intentionally override exit() in a common intermediate fragment subclass since the default behaviour in PApplet.exit() is to ultimately call [not-overridable, package protected method] exit2() which calls System.exit(). Nasty. onBackPressed() in PApplet calls exit().

In both cases rendering is ok and does not produce any errors. In one case there are no memory issues whatsoever, but in the other case there is a significant issue where the Activity can never be GC'd.

In that problematic case, the Activity instance is never GC'd even after finish() and onDestroy() have been called on that activity instance (as observed by sitting on the debugger).

When I look through the hprof capture (using IntelliJ), I can see that my activity instance is still held as a reference by the activity field in the PApplet superclass of the fragment:
LIVE REFERENCE TREE:
mpkg.processing.MyProcessingActivity@315940896 (0x12d4e020)
----(*PApplet.activity* field in) mypkg.processing.MyTroublesomeProcessingFragment@317768448 (0x12f0c300)

Of course I expect that the Activity and its dependants will have been marked for GC, but they are definitely not since the PApplet subclass fragment instance remains in memory and holds the ref to the activity.

Is it possible that I am somehow leaving the PApplet in a state such that it does not release the Activity reference that is held in (private) field activity ? Could these subtle changes

  1. to avoid System.exit(), and
  2. to set a 3D canvas
    be somehow responsible for this ?

I have no console errors or exceptions.
The troublesome fragment's draw() consist of a set of calls to translate(), stroke(), line(), ellipse(), fill().

My subclass fragment MyTroublesomeProcessingFragment does not hold any additional Activity refs.

@codeanticode codeanticode self-assigned this May 5, 2016
@codeanticode
Copy link
Contributor

codeanticode commented May 5, 2016

Well, the activity instance is also used to initialize the GLSurfaceView, (see here), so it could be the that another reference is kept inside it?

We could set the activity field to null when the sketch is disposed somewhere in here.

@jsalerno
Copy link
Author

jsalerno commented May 6, 2016

Hi, thanks for the feedback.
I had approximately the same thoughts, but have not yet contributed to the codebase.

Might it also be worthwhile in the final dispose method of PApplet to

surfaceView.getHolder().getSurface().release();

and then

surfaceView=null;

as well as

activity=null;

I could make the changes and issue a merge request.

A question then about the Ant build though ....
I tried to ant all at the project root after pulling from https://github.com/processing/processing-android.git , but could not get the process to build even the core. If you could point me to some detailed instructions, that would be great. Alternatively, I am happy to create a maven pom and build for the project. Please let me know if this would be useful.

One final comment ... I just downloaded the latest pre-built Processing IDE and installed the latest Android add-on version Android Mode 3.0-RC3 and if I happen to use that library with my own project (as opposed to the 3.0.1 or 3.0.2 libraries emitted from earlier IDE-managed projects) then I get essentially no rendering of either 2D or 3D views. That being the case, I would like to apply changes to an earlier version (I think it might be tag android-0246, prettyVersion=3.0-beta4). If I do that and I have my own fork, then the changes will need to be merged back to develop from the branch I create. Is that ok ?

@codeanticode
Copy link
Contributor

Yes, please create a PR, so I can look into it and merge if everything is ok.

As for building the android mode, please note that it depends on both the processing-core and the processing-java projects, which are inside the main processing repo. Note that the build scripts for android-core and the mode point to the main processing repo (here and here), and that you also need an ANDROID_SDK environmental variable holding the location of your SDK, with a copy of platform 15 (Android 4.0.3) in it. If all of this is in place, you should be able to run ant dist in the root folder, and get a zip package of the mode inside the release folder.

As for the use of the android-core library in your own (I assume) Eclipse or Android Studio project, please note that we made recent changes that turned PApplet a Fragment, so you need to create the main activity yourself. See these sample projects for a reference. More details about using Android Studio or Eclipse for development are available in the wiki.

@codeanticode
Copy link
Contributor

I added our suggestions for the dispose method in the latest release, RC4:

https://github.com/processing/processing-android/releases/tag/latest

Please let me know if it works.

@jsalerno
Copy link
Author

That change has fixed the Acivity-ref problem.
Thank you very much !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants