Giru Bhai
Giru Bhai

Reputation: 14398

Fix activity leak when listener keeps implicit reference

In the MessageFeedActivity onCreate method it load feeds by calling getMessageTypes method of CTFeedAPI class.

public class MessageFeedActivity extends AppCompatActivity {
 @Override
 protected void onCreate(Bundle savedInstanceState) {
  super.onCreate(savedInstanceState);

  //Setting the listener
  CTFeedAPI ctFeedAPI = new CTFeedAPI(new CTFeedAPI.CTFeedAPIListener() {
   @Override
   public void feedAPISuccessListener(Object object) {
    // Handle Success
   }

   @Override
   public void feedAPIErrorListener(int error) {
    // Handle Error
   }
  });

  ctFeedAPI.getMessageTypes();
 }

 @Override
 protected void onDestroy() {
  super.onDestroy();
 }
}

and wait for CTFeedAPIListener response. And CTFeedAPI class make network request by calling performRequest method of NetworkRequest class as

public class CTFeedAPI implements NetworkListener {
 private CTFeedAPIListener apiListener;

 public CTFeedAPI(CTFeedAPIListener feedAPIListener) {
  apiListener = feedAPIListener;
 }

 public void getMessageTypes() {
  Map < String, String > params = new HashMap < > ();
  params.put("f", "GetMessageTypes");

  NetworkRequest networkRequest = new NetworkRequest(this);
  networkRequest.performRequest();
 }

 public interface CTFeedAPIListener {
  void feedAPISuccessListener(Object object);

  void feedAPIErrorListener(int error);
 }
}

and wait for NetworkListener response

public class NetworkRequest {
 private NetworkListener mListener;

 public interface NetworkListener {

  void networkReqSuccessListener(String cacheKey, String tag, String response);

  void networkReqErrorListener(String tag, int error);
 }
 public NetworkRequest(NetworkListener listener) {
  this.mListener = listener;
 }

 public void performRequest(
  // Perform Network Requests and respond as
  if (mListener != null) {
   if (success) {
    mListener.networkReqSuccessListener(getUrl(), getTag(), response);
   } else {
    mListener.networkReqErrorListener(getTag(), err_msg);
   }
  }
 }

When users press back key, before destroy the MessageFeedActivity, the system call 'onDestroy' method. And Unfortunately, because the background thread (performRequest method in NetworkRequest class) is still keep a reference to it, leak occurs.

So how to implement CTFeedAPIListener reference in MessageFeedActivity to remove leak.

Upvotes: 2

Views: 161

Answers (2)

Amani
Amani

Reputation: 3979

Weak reference objects, which do not prevent their referents from being made finalizable, finalized, and then reclaimed. Weak references are most often used to implement canonicalizing mappings.

Suppose that the garbage collector determines at a certain point in time that an object is weakly reachable. At that time it will atomically clear all weak references to that object and all weak references to any other weakly-reachable objects from which that object is reachable through a chain of strong and soft references. At the same time it will declare all of the formerly weakly-reachable objects to be finalizable. At the same time or at some later time it will enqueue those newly-cleared weak references that are registered with reference queues.

You can use Weak Reference:

import java.lang.ref.WeakReference;

public class NetworkRequest {
 public interface NetworkListener {
      void networkReqSuccessListener(String cacheKey, String tag, String response);
      void networkReqErrorListener(String tag, int error);
 }

 private WeakReference<NetworkListener> mListener;

 public NetworkRequest(NetworkListener listener) {
      this.mListener = new WeakReference<NetworkListener>(listener);
 }

 public void performRequest(){
  // Perform Network Requests and respond as
  NetworkListener listener = mListener.get();
  if (listener != null) {
     if (success) listener.networkReqSuccessListener(getUrl(), getTag(), response);
     else listener.networkReqErrorListener(getTag(), err_msg);
   }
  }
}

public class CTFeedAPI implements NetworkListener {
 private WeakReference<CTFeedAPIListener> apiListener;

 public CTFeedAPI(CTFeedAPIListener feedAPIListener) {
  apiListener = new WeakReference<>(feedAPIListener);
 }

 public void getMessageTypes() {
  Map < String, String > params = new HashMap < > ();
  params.put("f", "GetMessageTypes");

  NetworkRequest networkRequest = new NetworkRequest(this);
  networkRequest.performRequest();
 }

 public interface CTFeedAPIListener {
  void feedAPISuccessListener(Object object);

  void feedAPIErrorListener(int error);
 }
}

save CTFeedAPI and CTFeedAPIListener as instance variable of MessageFeedActivity to prevent GC collecting them when activity is presented:

public class MessageFeedActivity extends AppCompatActivity {

private CTFeedAPI ctFeedAPI = null;// keeping a reference to CTFeedAPI
private CTFeedAPIListener listener = null;// keeping a reference to listener
 @Override
 protected void onCreate(Bundle savedInstanceState) {
  super.onCreate(savedInstanceState);

  //Setting the listener
  listener = new CTFeedAPI.CTFeedAPIListener() {
   @Override
   public void feedAPISuccessListener(Object object) {
    // Handle Success
   }

   @Override
   public void feedAPIErrorListener(int error) {
    // Handle Error
   }
  });
  ctFeedAPI = new CTFeedAPI(listener);
  ctFeedAPI.getMessageTypes();
 }

Upvotes: 1

The_Martian
The_Martian

Reputation: 3767

In this design not only you will leak memory but also your code would be highly coupled and very hard to test; prone to bugs that are hard to detect. I would suggest you implement MVP or similar architecture. Your activity should never know anything about your network layer. Add a presenter layer that is responsible to request something on behalf of your activity and use interface to update your activity. Your presenter should access a business entity that is mapped from the response of repository layer, that is responsible for network or Db access and return values to the client presenter. This way your presenter and business logic layers would be decoupled and easy to test independently. In the future if business requirements change, your changes don't affect other layers. Please see this article for more information on the subject.

Upvotes: 2

Related Questions