Skip to content

Commit 562f1bd

Browse files
zhanghaiGanesh Olekar
authored andcommitted
DO NOT MERGE Re-implement reading/writing Throwables from/to Parcel, without
Parcel private APIs. Bug:197228210 Test: atest CtsSecurityTestCases:android.security.cts.AndroidFutureTest (cherry picked from I577da5a3bc4ed537123b7eceaa5addf8f7bb0d92 and Icc5ce702f0cd84e9136dee3c65f63619df697358) Change-Id: I1d488c475f2f7af835a67496535cecdd6987c0cf
1 parent c9bc394 commit 562f1bd

File tree

2 files changed

+86
-58
lines changed

2 files changed

+86
-58
lines changed

core/java/com/android/internal/infra/AndroidFuture.java

Lines changed: 76 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,21 @@
1616

1717
package com.android.internal.infra;
1818

19-
import static com.android.internal.util.ConcurrentUtils.DIRECT_EXECUTOR;
20-
2119
import android.annotation.CallSuper;
2220
import android.annotation.NonNull;
2321
import android.annotation.Nullable;
2422
import android.os.Handler;
25-
import android.os.Message;
23+
import android.os.Looper;
2624
import android.os.Parcel;
2725
import android.os.Parcelable;
2826
import android.os.RemoteException;
29-
import android.util.ExceptionUtils;
27+
import android.util.EventLog;
3028
import android.util.Log;
3129

3230
import com.android.internal.annotations.GuardedBy;
3331
import com.android.internal.util.Preconditions;
34-
import com.android.internal.util.function.pooled.PooledLambda;
3532

33+
import java.lang.reflect.Constructor;
3634
import java.util.concurrent.CancellationException;
3735
import java.util.concurrent.CompletableFuture;
3836
import java.util.concurrent.CompletionStage;
@@ -75,14 +73,16 @@ public class AndroidFuture<T> extends CompletableFuture<T> implements Parcelable
7573

7674
private static final boolean DEBUG = false;
7775
private static final String LOG_TAG = AndroidFuture.class.getSimpleName();
76+
private static final Executor DIRECT_EXECUTOR = Runnable::run;
7877
private static final StackTraceElement[] EMPTY_STACK_TRACE = new StackTraceElement[0];
78+
private static @Nullable Handler sMainHandler;
7979

8080
private final @NonNull Object mLock = new Object();
8181
@GuardedBy("mLock")
8282
private @Nullable BiConsumer<? super T, ? super Throwable> mListener;
8383
@GuardedBy("mLock")
8484
private @Nullable Executor mListenerExecutor = DIRECT_EXECUTOR;
85-
private @NonNull Handler mTimeoutHandler = Handler.getMain();
85+
private @NonNull Handler mTimeoutHandler = getMainHandler();
8686
private final @Nullable IAndroidFuture mRemoteOrigin;
8787

8888
public AndroidFuture() {
@@ -96,7 +96,7 @@ public AndroidFuture() {
9696
// Done
9797
if (in.readBoolean()) {
9898
// Failed
99-
completeExceptionally(unparcelException(in));
99+
completeExceptionally(readThrowable(in));
100100
} else {
101101
// Success
102102
complete((T) in.readValue(null));
@@ -108,6 +108,15 @@ public AndroidFuture() {
108108
}
109109
}
110110

111+
@NonNull
112+
private static Handler getMainHandler() {
113+
// This isn't thread-safe but we are okay with it.
114+
if (sMainHandler == null) {
115+
sMainHandler = new Handler(Looper.getMainLooper());
116+
}
117+
return sMainHandler;
118+
}
119+
111120
/**
112121
* Create a completed future with the given value.
113122
*
@@ -236,9 +245,7 @@ private void callListenerAsync(BiConsumer<? super T, ? super Throwable> listener
236245
if (mListenerExecutor == DIRECT_EXECUTOR) {
237246
callListener(listener, res, err);
238247
} else {
239-
mListenerExecutor.execute(PooledLambda
240-
.obtainRunnable(AndroidFuture::callListener, listener, res, err)
241-
.recycleOnUse());
248+
mListenerExecutor.execute(() -> callListener(listener, res, err));
242249
}
243250
}
244251

@@ -260,7 +267,8 @@ static <TT> void callListener(
260267
} else {
261268
// listener exception-case threw
262269
// give up on listener but preserve the original exception when throwing up
263-
throw ExceptionUtils.appendCause(t, err);
270+
t.addSuppressed(err);
271+
throw t;
264272
}
265273
}
266274
} catch (Throwable t2) {
@@ -272,9 +280,7 @@ static <TT> void callListener(
272280
/** @inheritDoc */
273281
//@Override //TODO uncomment once java 9 APIs are exposed to frameworks
274282
public AndroidFuture<T> orTimeout(long timeout, @NonNull TimeUnit unit) {
275-
Message msg = PooledLambda.obtainMessage(AndroidFuture::triggerTimeout, this);
276-
msg.obj = this;
277-
mTimeoutHandler.sendMessageDelayed(msg, unit.toMillis(timeout));
283+
mTimeoutHandler.postDelayed(this::triggerTimeout, this, unit.toMillis(timeout));
278284
return this;
279285
}
280286

@@ -507,7 +513,7 @@ public void writeToParcel(Parcel dest, int flags) {
507513
result = get();
508514
} catch (Throwable t) {
509515
dest.writeBoolean(true);
510-
parcelException(dest, unwrapExecutionException(t));
516+
writeThrowable(dest, unwrapExecutionException(t));
511517
return;
512518
}
513519
dest.writeBoolean(false);
@@ -545,45 +551,75 @@ Throwable unwrapExecutionException(Throwable t) {
545551
* Alternative to {@link Parcel#writeException} that stores the stack trace, in a
546552
* way consistent with the binder IPC exception propagation behavior.
547553
*/
548-
private static void parcelException(Parcel p, @Nullable Throwable t) {
549-
p.writeBoolean(t == null);
550-
if (t == null) {
554+
private static void writeThrowable(@NonNull Parcel parcel, @Nullable Throwable throwable) {
555+
boolean hasThrowable = throwable != null;
556+
parcel.writeBoolean(hasThrowable);
557+
if (!hasThrowable) {
558+
return;
559+
}
560+
561+
boolean isFrameworkParcelable = throwable instanceof Parcelable
562+
&& throwable.getClass().getClassLoader() == Parcelable.class.getClassLoader();
563+
parcel.writeBoolean(isFrameworkParcelable);
564+
if (isFrameworkParcelable) {
565+
parcel.writeParcelable((Parcelable) throwable,
566+
Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
551567
return;
552568
}
553569

554-
p.writeInt(Parcel.getExceptionCode(t));
555-
p.writeString(t.getClass().getName());
556-
p.writeString(t.getMessage());
557-
p.writeStackTrace(t);
558-
parcelException(p, t.getCause());
570+
parcel.writeString(throwable.getClass().getName());
571+
parcel.writeString(throwable.getMessage());
572+
StackTraceElement[] stackTrace = throwable.getStackTrace();
573+
StringBuilder stackTraceBuilder = new StringBuilder();
574+
int truncatedStackTraceLength = Math.min(stackTrace != null ? stackTrace.length : 0, 5);
575+
for (int i = 0; i < truncatedStackTraceLength; i++) {
576+
if (i > 0) {
577+
stackTraceBuilder.append('\n');
578+
}
579+
stackTraceBuilder.append("\tat ").append(stackTrace[i]);
580+
}
581+
parcel.writeString(stackTraceBuilder.toString());
582+
writeThrowable(parcel, throwable.getCause());
559583
}
560584

561585
/**
562-
* @see #parcelException
586+
* @see #writeThrowable
563587
*/
564-
private static @Nullable Throwable unparcelException(Parcel p) {
565-
if (p.readBoolean()) {
588+
private static @Nullable Throwable readThrowable(@NonNull Parcel parcel) {
589+
final boolean hasThrowable = parcel.readBoolean();
590+
if (!hasThrowable) {
566591
return null;
567592
}
568593

569-
int exCode = p.readInt();
570-
String cls = p.readString();
571-
String msg = p.readString();
572-
String stackTrace = p.readInt() > 0 ? p.readString() : "\t<stack trace unavailable>";
573-
msg += "\n" + stackTrace;
574-
575-
Exception ex = p.createExceptionOrNull(exCode, msg);
576-
if (ex == null) {
577-
ex = new RuntimeException(cls + ": " + msg);
594+
boolean isFrameworkParcelable = parcel.readBoolean();
595+
if (isFrameworkParcelable) {
596+
return parcel.readParcelable(Parcelable.class.getClassLoader());
578597
}
579-
ex.setStackTrace(EMPTY_STACK_TRACE);
580598

581-
Throwable cause = unparcelException(p);
599+
String className = parcel.readString();
600+
String message = parcel.readString();
601+
String stackTrace = parcel.readString();
602+
String messageWithStackTrace = message + '\n' + stackTrace;
603+
Throwable throwable;
604+
try {
605+
Class<?> clazz = Class.forName(className, true, Parcelable.class.getClassLoader());
606+
if (Throwable.class.isAssignableFrom(clazz)) {
607+
Constructor<?> constructor = clazz.getConstructor(String.class);
608+
throwable = (Throwable) constructor.newInstance(messageWithStackTrace);
609+
} else {
610+
android.util.EventLog.writeEvent(0x534e4554, "186530450", -1, "");
611+
throwable = new RuntimeException(className + ": " + messageWithStackTrace);
612+
}
613+
} catch (Throwable t) {
614+
throwable = new RuntimeException(className + ": " + messageWithStackTrace);
615+
throwable.addSuppressed(t);
616+
}
617+
throwable.setStackTrace(EMPTY_STACK_TRACE);
618+
Throwable cause = readThrowable(parcel);
582619
if (cause != null) {
583-
ex.initCause(ex);
620+
throwable.initCause(cause);
584621
}
585-
586-
return ex;
622+
return throwable;
587623
}
588624

589625
@Override

core/java/com/android/internal/infra/ServiceConnector.java

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,11 @@
2626
import android.os.Handler;
2727
import android.os.IBinder;
2828
import android.os.IInterface;
29+
import android.os.Looper;
2930
import android.os.RemoteException;
3031
import android.os.UserHandle;
31-
import android.text.TextUtils;
32-
import android.util.DebugUtils;
3332
import android.util.Log;
3433

35-
import com.android.internal.util.function.pooled.PooledLambda;
36-
3734
import java.io.PrintWriter;
3835
import java.util.ArrayDeque;
3936
import java.util.ArrayList;
@@ -47,7 +44,6 @@
4744
import java.util.function.BiConsumer;
4845
import java.util.function.Function;
4946

50-
5147
/**
5248
* Takes care of managing a {@link ServiceConnection} and auto-disconnecting from the service upon
5349
* a certain timeout.
@@ -220,6 +216,7 @@ class Impl<I extends IInterface> extends ArrayDeque<Job<I, ?>>
220216
private final @NonNull Queue<Job<I, ?>> mQueue = this;
221217
private final @NonNull List<CompletionAwareJob<I, ?>> mUnfinishedJobs = new ArrayList<>();
222218

219+
private final @NonNull Handler mMainHandler = new Handler(Looper.getMainLooper());
223220
private final @NonNull ServiceConnection mServiceConnection = this;
224221
private final @NonNull Runnable mTimeoutDisconnect = this;
225222

@@ -250,9 +247,8 @@ class Impl<I extends IInterface> extends ArrayDeque<Job<I, ?>>
250247
* {@link IInterface}.
251248
* Typically this is {@code IMyInterface.Stub::asInterface}
252249
*/
253-
public Impl(@NonNull Context context, @NonNull Intent intent,
254-
@Context.BindServiceFlags int bindingFlags, @UserIdInt int userId,
255-
@Nullable Function<IBinder, I> binderAsInterface) {
250+
public Impl(@NonNull Context context, @NonNull Intent intent, int bindingFlags,
251+
@UserIdInt int userId, @Nullable Function<IBinder, I> binderAsInterface) {
256252
mContext = context;
257253
mIntent = intent;
258254
mBindingFlags = bindingFlags;
@@ -264,7 +260,7 @@ public Impl(@NonNull Context context, @NonNull Intent intent,
264260
* {@link Handler} on which {@link Job}s will be called
265261
*/
266262
protected Handler getJobHandler() {
267-
return Handler.getMain();
263+
return mMainHandler;
268264
}
269265

270266
/**
@@ -391,8 +387,7 @@ private void enqueue(@NonNull CompletionAwareJob<I, ?> task) {
391387

392388
private boolean enqueue(@NonNull Job<I, ?> job) {
393389
cancelTimeout();
394-
return getJobHandler().sendMessage(PooledLambda.obtainMessage(
395-
ServiceConnector.Impl::enqueueJobThread, this, job));
390+
return getJobHandler().post(() -> enqueueJobThread(job));
396391
}
397392

398393
void enqueueJobThread(@NonNull Job<I, ?> job) {
@@ -422,7 +417,7 @@ private void cancelTimeout() {
422417
if (DEBUG) {
423418
logTrace();
424419
}
425-
Handler.getMain().removeCallbacks(mTimeoutDisconnect);
420+
mMainHandler.removeCallbacks(mTimeoutDisconnect);
426421
}
427422

428423
void completeExceptionally(@NonNull Job<?, ?> job, @NonNull Throwable ex) {
@@ -486,7 +481,7 @@ private void scheduleUnbindTimeout() {
486481
}
487482
long timeout = getAutoDisconnectTimeoutMs();
488483
if (timeout > 0) {
489-
Handler.getMain().postDelayed(mTimeoutDisconnect, timeout);
484+
mMainHandler.postDelayed(mTimeoutDisconnect, timeout);
490485
} else if (DEBUG) {
491486
Log.i(LOG_TAG, "Not scheduling unbind for permanently bound " + this);
492487
}
@@ -502,7 +497,7 @@ public void unbind() {
502497
logTrace();
503498
}
504499
mUnbinding = true;
505-
getJobHandler().sendMessage(PooledLambda.obtainMessage(Impl::unbindJobThread, this));
500+
getJobHandler().post(this::unbindJobThread);
506501
}
507502

508503
void unbindJobThread() {
@@ -659,10 +654,7 @@ private String stateToString() {
659654
}
660655

661656
private void logTrace() {
662-
Log.i(LOG_TAG,
663-
TextUtils.join(" -> ",
664-
DebugUtils.callersWithin(ServiceConnector.class, /* offset= */ 1))
665-
+ "(" + this + ")");
657+
Log.i(LOG_TAG, "See stacktrace", new Throwable());
666658
}
667659

668660
/**

0 commit comments

Comments
 (0)