Do not use WeakReference for listeners (fixes #328).

This makes sure listeners are not invoked after being destroyed.

Also call onApiChange() on GUI thread.
This commit is contained in:
Felix Ableitner 2015-04-25 01:50:08 +02:00
parent 7ddee2f953
commit e821d2e468
6 changed files with 68 additions and 30 deletions

View File

@ -109,6 +109,12 @@ public class FolderPickerActivity extends SyncthingActivity
getService().registerOnApiChangeListener(this); getService().registerOnApiChangeListener(this);
} }
@Override
protected void onDestroy() {
super.onDestroy();
getService().unregisterOnApiChangeListener(this);
}
@Override @Override
public boolean onCreateOptionsMenu(Menu menu) { public boolean onCreateOptionsMenu(Menu menu) {
if (mListView.getAdapter() == mRootsAdapter) if (mListView.getAdapter() == mRootsAdapter)

View File

@ -44,8 +44,6 @@ public class MainActivity extends SyncthingActivity
private AlertDialog mDisabledDialog; private AlertDialog mDisabledDialog;
private boolean mIsDestroyed = false;
/** /**
* Causes population of folder and device lists, unlocks info drawer. * Causes population of folder and device lists, unlocks info drawer.
*/ */
@ -55,7 +53,7 @@ public class MainActivity extends SyncthingActivity
runOnUiThread(new Runnable() { runOnUiThread(new Runnable() {
@Override @Override
public void run() { public void run() {
if (currentState != SyncthingService.State.ACTIVE && !isFinishing() && !mIsDestroyed) { if (currentState != SyncthingService.State.ACTIVE && !isFinishing()) {
if (currentState == SyncthingService.State.DISABLED) { if (currentState == SyncthingService.State.DISABLED) {
if (mLoadingDialog != null) { if (mLoadingDialog != null) {
mLoadingDialog.dismiss(); mLoadingDialog.dismiss();
@ -215,7 +213,9 @@ public class MainActivity extends SyncthingActivity
if (mLoadingDialog != null) { if (mLoadingDialog != null) {
mLoadingDialog.dismiss(); mLoadingDialog.dismiss();
} }
mIsDestroyed = true; getService().unregisterOnApiChangeListener(this);
getService().unregisterOnApiChangeListener(mFolderFragment);
getService().unregisterOnApiChangeListener(mDevicesFragment);
} }
@Override @Override

View File

@ -125,17 +125,21 @@ public class DeviceSettingsFragment extends PreferenceFragment implements
} }
@Override @Override
public void onApiChange(SyncthingService.State currentState) { public void onDestroy() {
if (getActivity() == null || getActivity().isFinishing()) { super.onDestroy();
return; mSyncthingService.unregisterOnApiChangeListener(this);
}
else if (currentState != SyncthingService.State.ACTIVE) {
getActivity().finish();
} }
if (mIsCreate) { @Override
public void onApiChange(SyncthingService.State currentState) {
if (currentState != SyncthingService.State.ACTIVE) {
getActivity().finish();
return;
}
if (mIsCreate)
getActivity().setTitle(R.string.add_device); getActivity().setTitle(R.string.add_device);
} else { else {
RestApi.Device device = null; RestApi.Device device = null;
getActivity().setTitle(R.string.edit_device); getActivity().setTitle(R.string.edit_device);
List<RestApi.Device> devices = mSyncthingService.getApi().getDevices(false); List<RestApi.Device> devices = mSyncthingService.getApi().getDevices(false);

View File

@ -121,10 +121,7 @@ public class FolderSettingsFragment extends PreferenceFragment
@Override @Override
public void onApiChange(SyncthingService.State currentState) { public void onApiChange(SyncthingService.State currentState) {
if (getActivity() == null || getActivity().isFinishing()) { if (currentState != SyncthingService.State.ACTIVE) {
return;
}
else if (currentState != SyncthingService.State.ACTIVE) {
getActivity().finish(); getActivity().finish();
return; return;
} }
@ -185,6 +182,12 @@ public class FolderSettingsFragment extends PreferenceFragment
mSyncthingService.registerOnApiChangeListener(this); mSyncthingService.registerOnApiChangeListener(this);
} }
@Override
public void onDestroy() {
super.onDestroy();
mSyncthingService.unregisterOnApiChangeListener(this);
}
@Override @Override
public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) { public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) {
super.onCreateOptionsMenu(menu, inflater); super.onCreateOptionsMenu(menu, inflater);

View File

@ -160,6 +160,12 @@ public class SettingsFragment extends PreferenceFragment
mSyncthingService.registerOnApiChangeListener(this); mSyncthingService.registerOnApiChangeListener(this);
} }
@Override
public void onDestroy() {
super.onDestroy();
mSyncthingService.unregisterOnApiChangeListener(this);
}
/** /**
* Handles ActionBar back selected. * Handles ActionBar back selected.
*/ */

View File

@ -14,6 +14,7 @@ import android.content.SharedPreferences;
import android.os.AsyncTask; import android.os.AsyncTask;
import android.os.Build; import android.os.Build;
import android.os.Environment; import android.os.Environment;
import android.os.Handler;
import android.os.IBinder; import android.os.IBinder;
import android.preference.PreferenceManager; import android.preference.PreferenceManager;
import android.support.v4.app.NotificationCompat; import android.support.v4.app.NotificationCompat;
@ -31,7 +32,6 @@ import java.io.File;
import java.io.FileInputStream; import java.io.FileInputStream;
import java.io.FileOutputStream; import java.io.FileOutputStream;
import java.io.IOException; import java.io.IOException;
import java.lang.ref.WeakReference;
import java.nio.channels.FileChannel; import java.nio.channels.FileChannel;
import java.security.SecureRandom; import java.security.SecureRandom;
import java.util.HashSet; import java.util.HashSet;
@ -92,6 +92,8 @@ public class SyncthingService extends Service {
private final SyncthingServiceBinder mBinder = new SyncthingServiceBinder(this); private final SyncthingServiceBinder mBinder = new SyncthingServiceBinder(this);
private Handler mMainHandler;
/** /**
* Callback for when the Syncthing web interface becomes first available after service start. * Callback for when the Syncthing web interface becomes first available after service start.
*/ */
@ -106,7 +108,7 @@ public class SyncthingService extends Service {
public void onApiChange(State currentState); public void onApiChange(State currentState);
} }
private final HashSet<WeakReference<OnApiChangeListener>> mOnApiChangeListeners = private final HashSet<OnApiChangeListener> mOnApiChangeListeners =
new HashSet<>(); new HashSet<>();
/** /**
@ -139,10 +141,10 @@ public class SyncthingService extends Service {
*/ */
@Override @Override
public int onStartCommand(Intent intent, int flags, int startId) { public int onStartCommand(Intent intent, int flags, int startId) {
// Just catch the empty intent and return. if (intent == null)
if (intent == null) { return START_STICKY;
}
else if (ACTION_RESTART.equals(intent.getAction()) && mCurrentState == State.ACTIVE) { if (ACTION_RESTART.equals(intent.getAction()) && mCurrentState == State.ACTIVE) {
mApi.shutdown(); mApi.shutdown();
mCurrentState = State.INIT; mCurrentState = State.INIT;
updateState(); updateState();
@ -253,6 +255,7 @@ public class SyncthingService extends Service {
.putString("gui_password", sb.toString()).commit(); .putString("gui_password", sb.toString()).commit();
} }
mMainHandler = new Handler();
mDeviceStateHolder = new DeviceStateHolder(SyncthingService.this); mDeviceStateHolder = new DeviceStateHolder(SyncthingService.this);
registerReceiver(mDeviceStateHolder, new IntentFilter(Intent.ACTION_BATTERY_CHANGED)); registerReceiver(mDeviceStateHolder, new IntentFilter(Intent.ACTION_BATTERY_CHANGED));
new StartupTask().execute(); new StartupTask().execute();
@ -347,13 +350,24 @@ public class SyncthingService extends Service {
* Register a listener for the syncthing API state changing. * Register a listener for the syncthing API state changing.
* *
* The listener is called immediately with the current state, and again whenever the state * The listener is called immediately with the current state, and again whenever the state
* changes. * changes. The call is always from the GUI thread.
*
* @see {@link #unregisterOnApiChangeListener}
*/ */
public void registerOnApiChangeListener(OnApiChangeListener listener) { public void registerOnApiChangeListener(OnApiChangeListener listener) {
// Make sure we don't send an invalid state or syncthing might show a "disabled" message // Make sure we don't send an invalid state or syncthing might show a "disabled" message
// when it's just starting up. // when it's just starting up.
listener.onApiChange(mCurrentState); listener.onApiChange(mCurrentState);
mOnApiChangeListeners.add(new WeakReference<>(listener)); mOnApiChangeListeners.add(listener);
}
/**
* Unregisters a previously registered listener.
*
* @see {@link #registerOnApiChangeListener}
*/
public void unregisterOnApiChangeListener(OnApiChangeListener listener) {
mOnApiChangeListeners.remove(listener);
} }
private class PollWebGuiAvailableTaskImpl extends PollWebGuiAvailableTask { private class PollWebGuiAvailableTaskImpl extends PollWebGuiAvailableTask {
@ -382,16 +396,21 @@ public class SyncthingService extends Service {
* Must only be called from SyncthingService or {@link RestApi}. * Must only be called from SyncthingService or {@link RestApi}.
*/ */
private void onApiChange() { private void onApiChange() {
for (Iterator<WeakReference<OnApiChangeListener>> i = mOnApiChangeListeners.iterator(); mMainHandler.post(new Runnable() {
@Override
public void run() {
for (Iterator<OnApiChangeListener> i = mOnApiChangeListeners.iterator();
i.hasNext(); ) { i.hasNext(); ) {
WeakReference<OnApiChangeListener> listener = i.next(); OnApiChangeListener listener = i.next();
if (listener.get() != null) { if (listener != null) {
listener.get().onApiChange(mCurrentState); listener.onApiChange(mCurrentState);
} else { } else {
i.remove(); i.remove();
} }
} }
} }
});
}
/** /**
* Dialog to be shown when attempting to start syncthing while it is disabled according * Dialog to be shown when attempting to start syncthing while it is disabled according