From 80b1c4dcc8c5c9494c98693100a782644a16042d Mon Sep 17 00:00:00 2001 From: Charles Munger Date: Tue, 12 Nov 2019 09:00:48 -0800 Subject: [PATCH] Deprecate implicit Handler constructors These have been the cause of bugs in the past, such as b/10150258, because it's not always obvious or guaranteed that a calling thread will always have a looper, or that it's actually the looper you want. Specifying the looper explicitly, even if it's just Looper.myLooper(), makes the code clearer. Since callback APIs generally should have Executor variants added, it might be nice to also deprecate the constructors or options that rely on the calling thread's looper, but since I don't have an inventory of all those I'm starting with this. Bug: 144042891 Test: No behavior changes Change-Id: I0e7ba80d6b89a866ea715c790dd602ef728bf5bd --- api/current.txt | 4 ++-- core/java/android/os/Handler.java | 32 +++++++++++++++++++++++++------ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/api/current.txt b/api/current.txt index 1a87b2257d279..44ec9b2c41f26 100644 --- a/api/current.txt +++ b/api/current.txt @@ -34546,8 +34546,8 @@ package android.os { } public class Handler { - ctor public Handler(); - ctor public Handler(@Nullable android.os.Handler.Callback); + ctor @Deprecated public Handler(); + ctor @Deprecated public Handler(@Nullable android.os.Handler.Callback); ctor public Handler(@NonNull android.os.Looper); ctor public Handler(@NonNull android.os.Looper, @Nullable android.os.Handler.Callback); method @NonNull public static android.os.Handler createAsync(@NonNull android.os.Looper); diff --git a/core/java/android/os/Handler.java b/core/java/android/os/Handler.java index 9af9edae9a3f5..a99bdabee1376 100644 --- a/core/java/android/os/Handler.java +++ b/core/java/android/os/Handler.java @@ -28,15 +28,14 @@ import java.lang.reflect.Modifier; * A Handler allows you to send and process {@link Message} and Runnable * objects associated with a thread's {@link MessageQueue}. Each Handler * instance is associated with a single thread and that thread's message - * queue. When you create a new Handler, it is bound to the thread / - * message queue of the thread that is creating it -- from that point on, - * it will deliver messages and runnables to that message queue and execute - * them as they come out of the message queue. - * + * queue. When you create a new Handler it is bound to a {@link Looper}. + * It will deliver messages and runnables to that Looper's message + * queue and execute them on that Looper's thread. + * *

There are two main uses for a Handler: (1) to schedule messages and * runnables to be executed at some point in the future; and (2) to enqueue * an action to be performed on a different thread than your own. - * + * *

Scheduling messages is accomplished with the * {@link #post}, {@link #postAtTime(Runnable, long)}, * {@link #postDelayed}, {@link #sendEmptyMessage}, @@ -114,7 +113,18 @@ public class Handler { * * If this thread does not have a looper, this handler won't be able to receive messages * so an exception is thrown. + * + * @deprecated Implicitly choosing a Looper during Handler construction can lead to bugs + * where operations are silently lost (if the Handler is not expecting new tasks and quits), + * crashes (if a handler is sometimes created on a thread without a Looper active), or race + * conditions, where the thread a handler is associated with is not what the author + * anticipated. Instead, use an {@link java.util.concurrent.Executor} or specify the Looper + * explicitly, using {@link Looper#getMainLooper}, {link android.view.View#getHandler}, or + * similar. If the implicit thread local behavior is required for compatibility, use + * {@code new Handler(Looper.myLooper())} to make it clear to readers. + * */ + @Deprecated public Handler() { this(null, false); } @@ -128,7 +138,17 @@ public class Handler { * so an exception is thrown. * * @param callback The callback interface in which to handle messages, or null. + * + * @deprecated Implicitly choosing a Looper during Handler construction can lead to bugs + * where operations are silently lost (if the Handler is not expecting new tasks and quits), + * crashes (if a handler is sometimes created on a thread without a Looper active), or race + * conditions, where the thread a handler is associated with is not what the author + * anticipated. Instead, use an {@link java.util.concurrent.Executor} or specify the Looper + * explicitly, using {@link Looper#getMainLooper}, {link android.view.View#getHandler}, or + * similar. If the implicit thread local behavior is required for compatibility, use + * {@code new Handler(Looper.myLooper(), callback)} to make it clear to readers. */ + @Deprecated public Handler(@Nullable Callback callback) { this(callback, false); }