From c1d4bdbdfbd8fcd10b60c80e6b920fa8cddfc1b6 Mon Sep 17 00:00:00 2001 From: Vavassor Date: Thu, 23 Feb 2017 19:32:45 -0500 Subject: [PATCH] Volley request leaks across activity/fragment boundaries should no longer be able to occur. Also, the singleton allows contexts to be cleaned up instead of holding onto a dead reference. --- .../com/keylesspalace/tusky/AccountActivity.java | 12 ++++++++++-- .../com/keylesspalace/tusky/AccountFragment.java | 10 +++++++++- .../java/com/keylesspalace/tusky/MainActivity.java | 3 ++- .../keylesspalace/tusky/NotificationsFragment.java | 9 ++++++++- .../keylesspalace/tusky/PullNotificationService.java | 9 ++++++++- .../main/java/com/keylesspalace/tusky/SFragment.java | 9 ++++++++- .../com/keylesspalace/tusky/TimelineFragment.java | 9 ++++++++- .../com/keylesspalace/tusky/VolleySingleton.java | 10 +++++++--- 8 files changed, 60 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/AccountActivity.java b/app/src/main/java/com/keylesspalace/tusky/AccountActivity.java index 83959bdec..32d05ac3d 100644 --- a/app/src/main/java/com/keylesspalace/tusky/AccountActivity.java +++ b/app/src/main/java/com/keylesspalace/tusky/AccountActivity.java @@ -31,7 +31,6 @@ import android.util.TypedValue; import android.view.Menu; import android.view.MenuItem; import android.view.View; -import android.view.ViewGroup; import android.widget.TextView; import com.android.volley.AuthFailureError; @@ -51,7 +50,7 @@ import java.util.HashMap; import java.util.Map; public class AccountActivity extends BaseActivity { - private static final String TAG = "AccountActivity"; // logging tag + private static final String TAG = "AccountActivity"; // Volley request tag and logging tag private String domain; private String accessToken; @@ -121,6 +120,12 @@ public class AccountActivity extends BaseActivity { } } + @Override + protected void onDestroy() { + VolleySingleton.getInstance(this).cancelAll(TAG); + super.onDestroy(); + } + private void obtainAccount() { String endpoint = String.format(getString(R.string.endpoint_accounts), accountId); String url = "https://" + domain + endpoint; @@ -151,6 +156,7 @@ public class AccountActivity extends BaseActivity { return headers; } }; + request.setTag(TAG); VolleySingleton.getInstance(this).addToRequestQueue(request); } @@ -241,6 +247,7 @@ public class AccountActivity extends BaseActivity { return headers; } }; + request.setTag(TAG); VolleySingleton.getInstance(this).addToRequestQueue(request); } @@ -301,6 +308,7 @@ public class AccountActivity extends BaseActivity { return headers; } }; + request.setTag(TAG); VolleySingleton.getInstance(this).addToRequestQueue(request); } diff --git a/app/src/main/java/com/keylesspalace/tusky/AccountFragment.java b/app/src/main/java/com/keylesspalace/tusky/AccountFragment.java index 750ed507d..c0ddbbd5c 100644 --- a/app/src/main/java/com/keylesspalace/tusky/AccountFragment.java +++ b/app/src/main/java/com/keylesspalace/tusky/AccountFragment.java @@ -46,7 +46,7 @@ import java.util.Map; public class AccountFragment extends Fragment implements AccountActionListener, FooterActionListener { - private static final String TAG = "Account"; // logging tag + private static final String TAG = "Account"; // logging tag and Volley request tag public enum Type { FOLLOWS, @@ -94,6 +94,12 @@ public class AccountFragment extends Fragment implements AccountActionListener, accessToken = preferences.getString("accessToken", null); } + @Override + public void onDestroy() { + VolleySingleton.getInstance(getContext()).cancelAll(TAG); + super.onDestroy(); + } + @Nullable @Override public View onCreateView(LayoutInflater inflater, @Nullable ViewGroup container, @@ -211,6 +217,7 @@ public class AccountFragment extends Fragment implements AccountActionListener, return headers; } }; + request.setTag(TAG); VolleySingleton.getInstance(getContext()).addToRequestQueue(request); } @@ -300,6 +307,7 @@ public class AccountFragment extends Fragment implements AccountActionListener, return headers; } }; + request.setTag(TAG); VolleySingleton.getInstance(getContext()).addToRequestQueue(request); } diff --git a/app/src/main/java/com/keylesspalace/tusky/MainActivity.java b/app/src/main/java/com/keylesspalace/tusky/MainActivity.java index 360cd62ea..5e9201def 100644 --- a/app/src/main/java/com/keylesspalace/tusky/MainActivity.java +++ b/app/src/main/java/com/keylesspalace/tusky/MainActivity.java @@ -43,7 +43,7 @@ import java.util.HashMap; import java.util.Map; public class MainActivity extends BaseActivity { - private static final String TAG = "MainActivity"; // logging tag + private static final String TAG = "MainActivity"; // logging tag and Volley request tag private AlarmManager alarmManager; private PendingIntent serviceAlarmIntent; @@ -141,6 +141,7 @@ public class MainActivity extends BaseActivity { return headers; } }; + request.setTag(TAG); VolleySingleton.getInstance(this).addToRequestQueue(request); } } diff --git a/app/src/main/java/com/keylesspalace/tusky/NotificationsFragment.java b/app/src/main/java/com/keylesspalace/tusky/NotificationsFragment.java index cd7425821..e18894ff7 100644 --- a/app/src/main/java/com/keylesspalace/tusky/NotificationsFragment.java +++ b/app/src/main/java/com/keylesspalace/tusky/NotificationsFragment.java @@ -42,7 +42,7 @@ import java.util.Map; public class NotificationsFragment extends SFragment implements SwipeRefreshLayout.OnRefreshListener, StatusActionListener, FooterActionListener { - private static final String TAG = "Notifications"; // logging tag + private static final String TAG = "Notifications"; // logging tag and Volley request tag private SwipeRefreshLayout swipeRefreshLayout; private RecyclerView recyclerView; @@ -58,6 +58,12 @@ public class NotificationsFragment extends SFragment implements return fragment; } + @Override + public void onDestroy() { + VolleySingleton.getInstance(getContext()).cancelAll(TAG); + super.onDestroy(); + } + @Nullable @Override public View onCreateView(LayoutInflater inflater, @Nullable ViewGroup container, @@ -157,6 +163,7 @@ public class NotificationsFragment extends SFragment implements return headers; } }; + request.setTag(TAG); VolleySingleton.getInstance(getContext()).addToRequestQueue(request); } diff --git a/app/src/main/java/com/keylesspalace/tusky/PullNotificationService.java b/app/src/main/java/com/keylesspalace/tusky/PullNotificationService.java index 13d75e75f..6cad1c177 100644 --- a/app/src/main/java/com/keylesspalace/tusky/PullNotificationService.java +++ b/app/src/main/java/com/keylesspalace/tusky/PullNotificationService.java @@ -44,12 +44,18 @@ import java.util.Map; public class PullNotificationService extends IntentService { private static final int NOTIFY_ID = 6; // This is an arbitrary number. - private static final String TAG = "PullNotifications"; + private static final String TAG = "PullNotifications"; // logging tag and Volley request tag public PullNotificationService() { super("Tusky Pull Notification Service"); } + @Override + public void onDestroy() { + VolleySingleton.getInstance(this).cancelAll(TAG); + super.onDestroy(); + } + @Override protected void onHandleIntent(Intent intent) { SharedPreferences preferences = getSharedPreferences( @@ -94,6 +100,7 @@ public class PullNotificationService extends IntentService { return headers; } }; + request.setTag(TAG); VolleySingleton.getInstance(this).addToRequestQueue(request); } diff --git a/app/src/main/java/com/keylesspalace/tusky/SFragment.java b/app/src/main/java/com/keylesspalace/tusky/SFragment.java index a139c5efc..7f9e17677 100644 --- a/app/src/main/java/com/keylesspalace/tusky/SFragment.java +++ b/app/src/main/java/com/keylesspalace/tusky/SFragment.java @@ -47,7 +47,7 @@ import java.util.Map; * overlap functionality. So, I'm momentarily leaving it and hopefully working on those will clear * up what needs to be where. */ public class SFragment extends Fragment { - private static final String TAG = "SFragment"; // logging tag + private static final String TAG = "SFragment"; // logging tag and Volley request tag protected String domain; protected String accessToken; @@ -66,6 +66,12 @@ public class SFragment extends Fragment { loggedInUsername = preferences.getString("loggedInAccountUsername", null); } + @Override + public void onDestroy() { + VolleySingleton.getInstance(getContext()).cancelAll(TAG); + super.onDestroy(); + } + protected void sendRequest( int method, String endpoint, JSONObject parameters, @Nullable Response.Listener responseListener) { @@ -92,6 +98,7 @@ public class SFragment extends Fragment { return headers; } }; + request.setTag(TAG); VolleySingleton.getInstance(getContext()).addToRequestQueue(request); } diff --git a/app/src/main/java/com/keylesspalace/tusky/TimelineFragment.java b/app/src/main/java/com/keylesspalace/tusky/TimelineFragment.java index f4e48da4d..142bd254e 100644 --- a/app/src/main/java/com/keylesspalace/tusky/TimelineFragment.java +++ b/app/src/main/java/com/keylesspalace/tusky/TimelineFragment.java @@ -41,7 +41,7 @@ import java.util.Map; public class TimelineFragment extends SFragment implements SwipeRefreshLayout.OnRefreshListener, StatusActionListener, FooterActionListener { - private static final String TAG = "Timeline"; // logging tag + private static final String TAG = "Timeline"; // logging tag and Volley request tag public enum Kind { HOME, @@ -78,6 +78,12 @@ public class TimelineFragment extends SFragment implements return fragment; } + @Override + public void onDestroy() { + VolleySingleton.getInstance(getContext()).cancelAll(TAG); + super.onDestroy(); + } + @Override public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { @@ -221,6 +227,7 @@ public class TimelineFragment extends SFragment implements return headers; } }; + request.setTag(TAG); VolleySingleton.getInstance(getContext()).addToRequestQueue(request); } diff --git a/app/src/main/java/com/keylesspalace/tusky/VolleySingleton.java b/app/src/main/java/com/keylesspalace/tusky/VolleySingleton.java index 7ef7ee948..c1807b981 100644 --- a/app/src/main/java/com/keylesspalace/tusky/VolleySingleton.java +++ b/app/src/main/java/com/keylesspalace/tusky/VolleySingleton.java @@ -24,14 +24,18 @@ import com.android.volley.RequestQueue; import com.android.volley.toolbox.ImageLoader; import com.android.volley.toolbox.Volley; +import java.lang.ref.WeakReference; + public class VolleySingleton { private static VolleySingleton instance; private RequestQueue requestQueue; private ImageLoader imageLoader; - private static Context context; + /* This is a weak reference to account for the case where it might be held onto beyond the + * lifetime of the Activity/Fragment/Service, so it can be cleaned up. */ + private static WeakReference context; private VolleySingleton(Context context) { - VolleySingleton.context = context; + VolleySingleton.context = new WeakReference<>(context); requestQueue = getRequestQueue(); imageLoader = new ImageLoader(requestQueue, new ImageLoader.ImageCache() { @@ -60,7 +64,7 @@ public class VolleySingleton { if (requestQueue == null) { /* getApplicationContext() is key, it keeps you from leaking the * Activity or BroadcastReceiver if someone passes one in. */ - requestQueue= Volley.newRequestQueue(context.getApplicationContext()); + requestQueue= Volley.newRequestQueue(context.get().getApplicationContext()); } return requestQueue; }