From f3d943270c92d8fa72ae10bd1ecf95fde62c4b45 Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Sat, 9 Sep 2017 23:46:48 -0700 Subject: [PATCH] Be more conservative with handlers and references Expiring message timers could end up leaking references and executing work even after their conversation item was no longer visible Maybe fixes #6898 // FREEBIE --- .../securesms/ConversationListItem.java | 14 ++--- .../securesms/MessageRecipientListItem.java | 13 ++--- .../RecipientPreferenceActivity.java | 11 +--- .../thoughtcrime/securesms/ShareListItem.java | 11 ++-- .../components/ExpirationTimerView.java | 54 +++++++++++-------- .../securesms/components/InputPanel.java | 9 ++-- .../webrtc/WebRtcIncomingCallOverlay.java | 25 ++++----- .../securesms/service/WebRtcCallService.java | 3 +- src/org/thoughtcrime/securesms/util/Util.java | 4 ++ 9 files changed, 67 insertions(+), 77 deletions(-) diff --git a/src/org/thoughtcrime/securesms/ConversationListItem.java b/src/org/thoughtcrime/securesms/ConversationListItem.java index 091bc1e2e9..75438e56cf 100644 --- a/src/org/thoughtcrime/securesms/ConversationListItem.java +++ b/src/org/thoughtcrime/securesms/ConversationListItem.java @@ -23,7 +23,6 @@ import android.graphics.Typeface; import android.graphics.drawable.RippleDrawable; import android.os.Build.VERSION; import android.os.Build.VERSION_CODES; -import android.os.Handler; import android.support.annotation.DrawableRes; import android.support.annotation.NonNull; import android.util.AttributeSet; @@ -42,6 +41,7 @@ import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientModifiedListener; import org.thoughtcrime.securesms.util.DateUtils; import org.thoughtcrime.securesms.util.ResUtil; +import org.thoughtcrime.securesms.util.Util; import org.thoughtcrime.securesms.util.ViewUtil; import java.util.Locale; @@ -83,7 +83,6 @@ public class ConversationListItem extends RelativeLayout private final @DrawableRes int readBackground; private final @DrawableRes int unreadBackround; - private final Handler handler = new Handler(); private int distributionType; public ConversationListItem(Context context) { @@ -236,13 +235,10 @@ public class ConversationListItem extends RelativeLayout @Override public void onModified(final Recipient recipient) { - handler.post(new Runnable() { - @Override - public void run() { - fromView.setText(recipient, read); - contactPhotoImage.setAvatar(recipient, true); - setRippleColor(recipient); - } + Util.runOnMain(() -> { + fromView.setText(recipient, read); + contactPhotoImage.setAvatar(recipient, true); + setRippleColor(recipient); }); } diff --git a/src/org/thoughtcrime/securesms/MessageRecipientListItem.java b/src/org/thoughtcrime/securesms/MessageRecipientListItem.java index 3c8a23c91e..97eb5f553c 100644 --- a/src/org/thoughtcrime/securesms/MessageRecipientListItem.java +++ b/src/org/thoughtcrime/securesms/MessageRecipientListItem.java @@ -18,7 +18,6 @@ package org.thoughtcrime.securesms; import android.content.Context; import android.os.AsyncTask; -import android.os.Handler; import android.text.TextUtils; import android.util.AttributeSet; import android.view.View; @@ -37,6 +36,7 @@ import org.thoughtcrime.securesms.database.model.MessageRecord; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientModifiedListener; import org.thoughtcrime.securesms.sms.MessageSender; +import org.thoughtcrime.securesms.util.Util; /** * A simple view to show the recipients of a message @@ -56,8 +56,6 @@ public class MessageRecipientListItem extends RelativeLayout private Button resendButton; private AvatarImageView contactPhotoImage; - private final Handler handler = new Handler(); - public MessageRecipientListItem(Context context) { super(context); } @@ -164,12 +162,9 @@ public class MessageRecipientListItem extends RelativeLayout @Override public void onModified(final Recipient recipient) { - handler.post(new Runnable() { - @Override - public void run() { - fromView.setText(recipient); - contactPhotoImage.setAvatar(recipient, false); - } + Util.runOnMain(() -> { + fromView.setText(recipient); + contactPhotoImage.setAvatar(recipient, false); }); } diff --git a/src/org/thoughtcrime/securesms/RecipientPreferenceActivity.java b/src/org/thoughtcrime/securesms/RecipientPreferenceActivity.java index 25945e5ed6..98f6955266 100644 --- a/src/org/thoughtcrime/securesms/RecipientPreferenceActivity.java +++ b/src/org/thoughtcrime/securesms/RecipientPreferenceActivity.java @@ -48,6 +48,7 @@ import org.thoughtcrime.securesms.util.DynamicLanguage; import org.thoughtcrime.securesms.util.DynamicNoActionBarTheme; import org.thoughtcrime.securesms.util.DynamicTheme; import org.thoughtcrime.securesms.util.IdentityUtil; +import org.thoughtcrime.securesms.util.Util; import org.thoughtcrime.securesms.util.concurrent.ListenableFuture; import org.whispersystems.libsignal.util.guava.Optional; @@ -189,9 +190,6 @@ public class RecipientPreferenceActivity extends PassphraseRequiredActionBarActi extends PreferenceFragment implements RecipientModifiedListener { - - private final Handler handler = new Handler(); - private Recipient recipient; private BroadcastReceiver staleReceiver; private MasterSecret masterSecret; @@ -329,12 +327,7 @@ public class RecipientPreferenceActivity extends PassphraseRequiredActionBarActi @Override public void onModified(final Recipient recipient) { - handler.post(new Runnable() { - @Override - public void run() { - setSummaries(recipient); - } - }); + Util.runOnMain(() -> setSummaries(recipient)); } private class RingtoneChangeListener implements Preference.OnPreferenceChangeListener { diff --git a/src/org/thoughtcrime/securesms/ShareListItem.java b/src/org/thoughtcrime/securesms/ShareListItem.java index 00777cc7a8..4ef1cbef31 100644 --- a/src/org/thoughtcrime/securesms/ShareListItem.java +++ b/src/org/thoughtcrime/securesms/ShareListItem.java @@ -27,6 +27,7 @@ import org.thoughtcrime.securesms.components.FromTextView; import org.thoughtcrime.securesms.database.model.ThreadRecord; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientModifiedListener; +import org.thoughtcrime.securesms.util.Util; /** * A simple view to show the recipients of an open conversation @@ -45,7 +46,6 @@ public class ShareListItem extends RelativeLayout private AvatarImageView contactPhotoImage; - private final Handler handler = new Handler(); private int distributionType; public ShareListItem(Context context) { @@ -104,12 +104,9 @@ public class ShareListItem extends RelativeLayout @Override public void onModified(final Recipient recipient) { - handler.post(new Runnable() { - @Override - public void run() { - fromView.setText(recipient); - contactPhotoImage.setAvatar(recipient, false); - } + Util.runOnMain(() -> { + fromView.setText(recipient); + contactPhotoImage.setAvatar(recipient, false); }); } } diff --git a/src/org/thoughtcrime/securesms/components/ExpirationTimerView.java b/src/org/thoughtcrime/securesms/components/ExpirationTimerView.java index 5e9deac6b0..4bd0d6ca99 100644 --- a/src/org/thoughtcrime/securesms/components/ExpirationTimerView.java +++ b/src/org/thoughtcrime/securesms/components/ExpirationTimerView.java @@ -1,16 +1,17 @@ package org.thoughtcrime.securesms.components; import android.content.Context; -import android.os.Handler; +import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.util.AttributeSet; +import org.thoughtcrime.securesms.util.Util; + +import java.lang.ref.WeakReference; import java.util.concurrent.TimeUnit; public class ExpirationTimerView extends HourglassView { - private final Handler handler = new Handler(); - private long startedAt; private long expiresIn; @@ -39,26 +40,11 @@ public class ExpirationTimerView extends HourglassView { public void startAnimation() { synchronized (this) { visible = true; - if (stopped == false) return; - else stopped = false; + if (!stopped) return; + else stopped = false; } - handler.postDelayed(new Runnable() { - @Override - public void run() { - setPercentage(calculateProgress(startedAt, expiresIn)); - - - synchronized (ExpirationTimerView.this) { - if (!visible) { - stopped = true; - return; - } - } - - handler.postDelayed(this, calculateAnimationDelay(startedAt, expiresIn)); - } - }, calculateAnimationDelay(this.startedAt, this.expiresIn)); + Util.runOnMainDelayed(new AnimationUpdateRunnable(this), calculateAnimationDelay(this.startedAt, this.expiresIn)); } public void stopAnimation() { @@ -85,4 +71,30 @@ public class ExpirationTimerView extends HourglassView { } } + private static class AnimationUpdateRunnable implements Runnable { + + private final WeakReference expirationTimerViewReference; + + private AnimationUpdateRunnable(@NonNull ExpirationTimerView expirationTimerView) { + this.expirationTimerViewReference = new WeakReference<>(expirationTimerView); + } + + @Override + public void run() { + ExpirationTimerView timerView = expirationTimerViewReference.get(); + if (timerView == null) return; + + timerView.setExpirationTime(timerView.startedAt, timerView.expiresIn); + + synchronized (timerView) { + if (!timerView.visible) { + timerView.stopped = true; + return; + } + } + + Util.runOnMainDelayed(this, timerView.calculateAnimationDelay(timerView.startedAt, timerView.expiresIn)); + } + } + } diff --git a/src/org/thoughtcrime/securesms/components/InputPanel.java b/src/org/thoughtcrime/securesms/components/InputPanel.java index 6ea8f5b441..321cca0aef 100644 --- a/src/org/thoughtcrime/securesms/components/InputPanel.java +++ b/src/org/thoughtcrime/securesms/components/InputPanel.java @@ -4,7 +4,6 @@ import android.annotation.TargetApi; import android.content.Context; import android.net.Uri; import android.os.Build; -import android.os.Handler; import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.v4.view.ViewCompat; @@ -25,11 +24,11 @@ import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.components.emoji.EmojiDrawer; import org.thoughtcrime.securesms.components.emoji.EmojiToggle; import org.thoughtcrime.securesms.util.TextSecurePreferences; +import org.thoughtcrime.securesms.util.Util; import org.thoughtcrime.securesms.util.ViewUtil; import org.thoughtcrime.securesms.util.concurrent.AssertedSuccessListener; import org.thoughtcrime.securesms.util.concurrent.ListenableFuture; import org.thoughtcrime.securesms.util.concurrent.SettableFuture; -import org.thoughtcrime.securesms.util.views.Stub; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; @@ -288,9 +287,7 @@ public class InputPanel extends LinearLayout private static class RecordTime implements Runnable { private final TextView recordTimeView; - private final AtomicLong startTime = new AtomicLong(0); - private final Handler handler = new Handler(); private RecordTime(TextView recordTimeView) { this.recordTimeView = recordTimeView; @@ -300,7 +297,7 @@ public class InputPanel extends LinearLayout this.startTime.set(System.currentTimeMillis()); this.recordTimeView.setText(DateUtils.formatElapsedTime(0)); ViewUtil.fadeIn(this.recordTimeView, FADE_TIME); - handler.postDelayed(this, TimeUnit.SECONDS.toMillis(1)); + Util.runOnMainDelayed(this, TimeUnit.SECONDS.toMillis(1)); } public long hide() { @@ -316,7 +313,7 @@ public class InputPanel extends LinearLayout if (localStartTime > 0) { long elapsedTime = System.currentTimeMillis() - localStartTime; recordTimeView.setText(DateUtils.formatElapsedTime(TimeUnit.MILLISECONDS.toSeconds(elapsedTime))); - handler.postDelayed(this, TimeUnit.SECONDS.toMillis(1)); + Util.runOnMainDelayed(this, TimeUnit.SECONDS.toMillis(1)); } } } diff --git a/src/org/thoughtcrime/securesms/components/webrtc/WebRtcIncomingCallOverlay.java b/src/org/thoughtcrime/securesms/components/webrtc/WebRtcIncomingCallOverlay.java index fa06b1c08d..8804fa52a4 100644 --- a/src/org/thoughtcrime/securesms/components/webrtc/WebRtcIncomingCallOverlay.java +++ b/src/org/thoughtcrime/securesms/components/webrtc/WebRtcIncomingCallOverlay.java @@ -1,8 +1,6 @@ package org.thoughtcrime.securesms.components.webrtc; import android.content.Context; -import android.os.Handler; -import android.os.Message; import android.support.annotation.Nullable; import android.util.AttributeSet; import android.view.LayoutInflater; @@ -11,8 +9,9 @@ import android.view.animation.Animation; import android.widget.RelativeLayout; import android.widget.TextView; -import org.thoughtcrime.securesms.components.multiwaveview.MultiWaveView; import org.thoughtcrime.securesms.R; +import org.thoughtcrime.securesms.components.multiwaveview.MultiWaveView; +import org.thoughtcrime.securesms.util.Util; /** * Displays the controls at the bottom of the in-call screen. @@ -26,16 +25,6 @@ public class WebRtcIncomingCallOverlay extends RelativeLayout { private MultiWaveView incomingCallWidget; private TextView redphoneLabel; - private Handler handler = new Handler() { - @Override - public void handleMessage(Message message) { - if (incomingCallWidget.getVisibility() == View.VISIBLE) { - incomingCallWidget.ping(); - handler.sendEmptyMessageDelayed(0, 1200); - } - } - }; - public WebRtcIncomingCallOverlay(Context context) { super(context); initialize(); @@ -63,7 +52,15 @@ public class WebRtcIncomingCallOverlay extends RelativeLayout { incomingCallWidget.setVisibility(View.VISIBLE); redphoneLabel.setVisibility(View.VISIBLE); - handler.sendEmptyMessageDelayed(0, 500); + Util.runOnMainDelayed(new Runnable() { + @Override + public void run() { + if (incomingCallWidget.getVisibility() == View.VISIBLE) { + incomingCallWidget.ping(); + Util.runOnMainDelayed(this, 1200); + } + } + }, 500); } public void setActiveCall() { diff --git a/src/org/thoughtcrime/securesms/service/WebRtcCallService.java b/src/org/thoughtcrime/securesms/service/WebRtcCallService.java index 6638e2afbd..39e4c3ccc0 100644 --- a/src/org/thoughtcrime/securesms/service/WebRtcCallService.java +++ b/src/org/thoughtcrime/securesms/service/WebRtcCallService.java @@ -147,7 +147,6 @@ public class WebRtcCallService extends Service implements InjectableType, PeerCo private boolean localVideoEnabled = false; private boolean remoteVideoEnabled = false; private boolean bluetoothAvailable = false; - private Handler serviceHandler = new Handler(); @Inject public SignalMessageSenderFactory messageSenderFactory; @Inject public SignalServiceAccountManager accountManager; @@ -611,7 +610,7 @@ public class WebRtcCallService extends Service implements InjectableType, PeerCo sendMessage(WebRtcViewModel.State.CALL_BUSY, recipient, localVideoEnabled, remoteVideoEnabled, bluetoothAvailable, microphoneEnabled); audioManager.startOutgoingRinger(OutgoingRinger.Type.BUSY); - serviceHandler.postDelayed(new Runnable() { + Util.runOnMainDelayed(new Runnable() { @Override public void run() { Intent intent = new Intent(WebRtcCallService.this, WebRtcCallService.class); diff --git a/src/org/thoughtcrime/securesms/util/Util.java b/src/org/thoughtcrime/securesms/util/Util.java index 2f247966fe..243082a0e5 100644 --- a/src/org/thoughtcrime/securesms/util/Util.java +++ b/src/org/thoughtcrime/securesms/util/Util.java @@ -379,6 +379,10 @@ public class Util { else handler.post(runnable); } + public static void runOnMainDelayed(final @NonNull Runnable runnable, long delayMillis) { + handler.postDelayed(runnable, delayMillis); + } + public static void runOnMainSync(final @NonNull Runnable runnable) { if (isMainThread()) { runnable.run();