Skip to content
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

Jump to bottom

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

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 1 commit
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.
Jump to
Jump to file
Failed to load files.
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
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);
}
}