MifareClassicTool icon indicating copy to clipboard operation
MifareClassicTool copied to clipboard

Activity memory leak caused by anonymous threads

Open cuixiaoyiyi opened this issue 1 year ago • 4 comments

Possible Memory Leak

An anonymous inner class will hold a reference to the this pointer of the outer class and will not be released until the thread ends. It will hold the Activity and prevent its timely release. Please check the links below.

Occurrences

https://github.com/ikarus23/MifareClassicTool/blob/master/Mifare%20Classic%20Tool/app/src/main/java/de/syss/MifareClassicTool/Activities/WriteTag.java#L1253 https://github.com/ikarus23/MifareClassicTool/blob/master/Mifare%20Classic%20Tool/app/src/main/java/de/syss/MifareClassicTool/Activities/KeyMapCreator.java#L458

Possible Solution

If it is necessary, it can be changed to static class + weak reference to eliminate the reference to the activity, which may cause memory leaks. Further discussion is welcome.

cuixiaoyiyi avatar Aug 31 '22 04:08 cuixiaoyiyi

Thanks. Not sure if this is an issue here. I want to keep the Activity while the thread/runner is working. I will try to have a closer look. Hints/tips are welcome. ;)

ikarus23 avatar Sep 05 '22 20:09 ikarus23

Any more hints? If I'm not mistaken, the behavior is by design. The anonymous class should be able to control the outer class (Activity). If the inner class was successful it finishes the outer class as well. If the inner class was unsuccessful it just ends his self (and with that it reference to the outer class?!).

I'm still not sure if this is an issue here. But I'm no Java programming professional, just a hobbyist.

ikarus23 avatar Aug 18 '23 08:08 ikarus23

** The anonymous class should be able to control the outer class (Activity). ** This is the exception case for anonymous threads, active (unfinished) thread objects are included in the GC Root, it will not be released until it finishes executing, and the objects it holds during this period cannot be released in time. The life cycle of Activity and anonymous thread in the link above cannot be guaranteed to be consistent. Activity.onDestroy() --> GC can/should recycle the activity The activity referenced by the thread --> GC can not recycle the activity (There is maybe a memory leak).

If the activity has been released by the manager in Android platform, a (WindowManager.BadTokenException) may happen after the Toast.makeText invocation.

cuixiaoyiyi avatar Aug 18 '23 09:08 cuixiaoyiyi

Solution: 
static ThreadA extends thread{  
     WeakReference<Activity>  w = null;
     // or 
     Context c = null; // getApplicationContext()  with long lifecyle
     void  run(){
	    	 try{
	                for/while(...){
	                      ....
	                      if(isInterrupted()){
	                          break;
	                      }
	                }
	                Activity a = w.get();
	                if(a != null ){
	                    // use a
	                }
	                //   or  
	                Toast.makeText(c,...);  // ApplicationContext is enough; Activity is unnecessary.
	           }catch(InterruptedException e){
	                 ....
	           }  
       }
}
class Activity:
void onDestroy(){
     ....
     if(thread != null && thread.isAlive()){
          thread .interrupt();
     }
}

cuixiaoyiyi avatar Aug 18 '23 09:08 cuixiaoyiyi