Skip to content

Enclose all operations in BinderTransport using obtained Parcels in try-finally blocks. #8733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 8, 2021
Merged

Enclose all operations in BinderTransport using obtained Parcels in try-finally blocks. #8733

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bf2d097
Enclose all operations using obtained Parcels in try-finally blocks t…
groakley Dec 1, 2021
ce75abe
Retrigger checks
groakley Dec 8, 2021
d207b58
Retrigger checks
groakley Dec 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 18 additions & 13 deletions binder/src/main/java/io/grpc/binder/internal/BinderTransport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,17 +323,18 @@ final void sendSetupTransaction() {
@GuardedBy("this")
final void sendSetupTransaction(IBinder iBinder) {
Parcel parcel = Parcel.obtain();
parcel.writeInt(WIRE_FORMAT_VERSION);
parcel.writeStrongBinder(incomingBinder);
try {
parcel.writeInt(WIRE_FORMAT_VERSION);
parcel.writeStrongBinder(incomingBinder);
if (!iBinder.transact(SETUP_TRANSPORT, parcel, null, IBinder.FLAG_ONEWAY)) {
shutdownInternal(
Status.UNAVAILABLE.withDescription("Failed sending SETUP_TRANSPORT transaction"), true);
}
} catch (RemoteException re) {
shutdownInternal(statusFromRemoteException(re), true);
} finally {
parcel.recycle();
}
parcel.recycle();
}

@GuardedBy("this")
Expand All @@ -345,14 +346,15 @@ private final void sendShutdownTransaction() {
// Ignore.
}
Parcel parcel = Parcel.obtain();
// Send empty flags to avoid a memory leak linked to empty parcels (b/207778694).
parcel.writeInt(0);
try {
// Send empty flags to avoid a memory leak linked to empty parcels (b/207778694).
parcel.writeInt(0);
outgoingBinder.transact(SHUTDOWN_TRANSPORT, parcel, null, IBinder.FLAG_ONEWAY);
} catch (RemoteException re) {
// Ignore.
} finally {
parcel.recycle();
}
parcel.recycle();
}
}

Expand All @@ -363,8 +365,8 @@ protected synchronized void sendPing(int id) throws StatusException {
throw Status.FAILED_PRECONDITION.withDescription("Transport not ready.").asException();
} else {
Parcel parcel = Parcel.obtain();
parcel.writeInt(id);
try {
parcel.writeInt(id);
outgoingBinder.transact(PING, parcel, null, IBinder.FLAG_ONEWAY);
} catch (RemoteException re) {
throw statusFromRemoteException(re).asException();
Expand Down Expand Up @@ -410,15 +412,16 @@ final void sendTransaction(int callId, Parcel parcel) throws StatusException {

final void sendOutOfBandClose(int callId, Status status) {
Parcel parcel = Parcel.obtain();
parcel.writeInt(0); // Placeholder for flags. Will be filled in below.
int flags = TransactionUtils.writeStatus(parcel, status);
TransactionUtils.fillInFlags(parcel, flags | TransactionUtils.FLAG_OUT_OF_BAND_CLOSE);
try {
parcel.writeInt(0); // Placeholder for flags. Will be filled in below.
int flags = TransactionUtils.writeStatus(parcel, status);
TransactionUtils.fillInFlags(parcel, flags | TransactionUtils.FLAG_OUT_OF_BAND_CLOSE);
sendTransaction(callId, parcel);
} catch (StatusException e) {
logger.log(Level.WARNING, "Failed sending oob close transaction", e);
} finally {
parcel.recycle();
}
parcel.recycle();
}

@Override
Expand Down Expand Up @@ -507,16 +510,17 @@ private void sendAcknowledgeBytes(IBinder iBinder) {
long n = numIncomingBytes.get();
acknowledgedIncomingBytes = n;
Parcel parcel = Parcel.obtain();
parcel.writeLong(n);
try {
parcel.writeLong(n);
if (!iBinder.transact(ACKNOWLEDGE_BYTES, parcel, null, IBinder.FLAG_ONEWAY)) {
shutdownInternal(
Status.UNAVAILABLE.withDescription("Failed sending ack bytes transaction"), true);
}
} catch (RemoteException re) {
shutdownInternal(statusFromRemoteException(re), true);
} finally {
parcel.recycle();
}
parcel.recycle();
}

@GuardedBy("this")
Expand Down Expand Up @@ -909,3 +913,4 @@ private static Status statusFromRemoteException(RemoteException e) {
return Status.INTERNAL.withCause(e);
}
}