Enclose all operations using obtained Parcels in try-finally blocks that will recycle the Parcel in the case that any exception is thrown. (#8733)

This commit is contained in:
Grant Oakley 2021-12-08 14:39:42 -08:00 committed by GitHub
parent 24330bccff
commit e28145ab6a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 18 additions and 13 deletions

View File

@ -323,18 +323,19 @@ public abstract class BinderTransport
@GuardedBy("this") @GuardedBy("this")
final void sendSetupTransaction(IBinder iBinder) { final void sendSetupTransaction(IBinder iBinder) {
Parcel parcel = Parcel.obtain(); Parcel parcel = Parcel.obtain();
try {
parcel.writeInt(WIRE_FORMAT_VERSION); parcel.writeInt(WIRE_FORMAT_VERSION);
parcel.writeStrongBinder(incomingBinder); parcel.writeStrongBinder(incomingBinder);
try {
if (!iBinder.transact(SETUP_TRANSPORT, parcel, null, IBinder.FLAG_ONEWAY)) { if (!iBinder.transact(SETUP_TRANSPORT, parcel, null, IBinder.FLAG_ONEWAY)) {
shutdownInternal( shutdownInternal(
Status.UNAVAILABLE.withDescription("Failed sending SETUP_TRANSPORT transaction"), true); Status.UNAVAILABLE.withDescription("Failed sending SETUP_TRANSPORT transaction"), true);
} }
} catch (RemoteException re) { } catch (RemoteException re) {
shutdownInternal(statusFromRemoteException(re), true); shutdownInternal(statusFromRemoteException(re), true);
} } finally {
parcel.recycle(); parcel.recycle();
} }
}
@GuardedBy("this") @GuardedBy("this")
private final void sendShutdownTransaction() { private final void sendShutdownTransaction() {
@ -345,16 +346,17 @@ public abstract class BinderTransport
// Ignore. // Ignore.
} }
Parcel parcel = Parcel.obtain(); Parcel parcel = Parcel.obtain();
try {
// Send empty flags to avoid a memory leak linked to empty parcels (b/207778694). // Send empty flags to avoid a memory leak linked to empty parcels (b/207778694).
parcel.writeInt(0); parcel.writeInt(0);
try {
outgoingBinder.transact(SHUTDOWN_TRANSPORT, parcel, null, IBinder.FLAG_ONEWAY); outgoingBinder.transact(SHUTDOWN_TRANSPORT, parcel, null, IBinder.FLAG_ONEWAY);
} catch (RemoteException re) { } catch (RemoteException re) {
// Ignore. // Ignore.
} } finally {
parcel.recycle(); parcel.recycle();
} }
} }
}
protected synchronized void sendPing(int id) throws StatusException { protected synchronized void sendPing(int id) throws StatusException {
if (inState(TransportState.SHUTDOWN_TERMINATED)) { if (inState(TransportState.SHUTDOWN_TERMINATED)) {
@ -363,8 +365,8 @@ public abstract class BinderTransport
throw Status.FAILED_PRECONDITION.withDescription("Transport not ready.").asException(); throw Status.FAILED_PRECONDITION.withDescription("Transport not ready.").asException();
} else { } else {
Parcel parcel = Parcel.obtain(); Parcel parcel = Parcel.obtain();
parcel.writeInt(id);
try { try {
parcel.writeInt(id);
outgoingBinder.transact(PING, parcel, null, IBinder.FLAG_ONEWAY); outgoingBinder.transact(PING, parcel, null, IBinder.FLAG_ONEWAY);
} catch (RemoteException re) { } catch (RemoteException re) {
throw statusFromRemoteException(re).asException(); throw statusFromRemoteException(re).asException();
@ -410,16 +412,17 @@ public abstract class BinderTransport
final void sendOutOfBandClose(int callId, Status status) { final void sendOutOfBandClose(int callId, Status status) {
Parcel parcel = Parcel.obtain(); Parcel parcel = Parcel.obtain();
try {
parcel.writeInt(0); // Placeholder for flags. Will be filled in below. parcel.writeInt(0); // Placeholder for flags. Will be filled in below.
int flags = TransactionUtils.writeStatus(parcel, status); int flags = TransactionUtils.writeStatus(parcel, status);
TransactionUtils.fillInFlags(parcel, flags | TransactionUtils.FLAG_OUT_OF_BAND_CLOSE); TransactionUtils.fillInFlags(parcel, flags | TransactionUtils.FLAG_OUT_OF_BAND_CLOSE);
try {
sendTransaction(callId, parcel); sendTransaction(callId, parcel);
} catch (StatusException e) { } catch (StatusException e) {
logger.log(Level.WARNING, "Failed sending oob close transaction", e); logger.log(Level.WARNING, "Failed sending oob close transaction", e);
} } finally {
parcel.recycle(); parcel.recycle();
} }
}
@Override @Override
public final boolean handleTransaction(int code, Parcel parcel) { public final boolean handleTransaction(int code, Parcel parcel) {
@ -507,17 +510,18 @@ public abstract class BinderTransport
long n = numIncomingBytes.get(); long n = numIncomingBytes.get();
acknowledgedIncomingBytes = n; acknowledgedIncomingBytes = n;
Parcel parcel = Parcel.obtain(); Parcel parcel = Parcel.obtain();
parcel.writeLong(n);
try { try {
parcel.writeLong(n);
if (!iBinder.transact(ACKNOWLEDGE_BYTES, parcel, null, IBinder.FLAG_ONEWAY)) { if (!iBinder.transact(ACKNOWLEDGE_BYTES, parcel, null, IBinder.FLAG_ONEWAY)) {
shutdownInternal( shutdownInternal(
Status.UNAVAILABLE.withDescription("Failed sending ack bytes transaction"), true); Status.UNAVAILABLE.withDescription("Failed sending ack bytes transaction"), true);
} }
} catch (RemoteException re) { } catch (RemoteException re) {
shutdownInternal(statusFromRemoteException(re), true); shutdownInternal(statusFromRemoteException(re), true);
} } finally {
parcel.recycle(); parcel.recycle();
} }
}
@GuardedBy("this") @GuardedBy("this")
final void handleAcknowledgedBytes(long numBytes) { final void handleAcknowledgedBytes(long numBytes) {
@ -909,3 +913,4 @@ public abstract class BinderTransport
return Status.INTERNAL.withCause(e); return Status.INTERNAL.withCause(e);
} }
} }