Enclose all operations using obtained Parcels in try-finally blocks t…

…hat will recycle the Parcel in the case that any exception is thrown. (#8733)
grpc · Dec 8, 2021 · e28145a · e28145a
1 parent 24330bc
commit e28145a
Showing 1 changed file with 18 additions and 13 deletions.
diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java b/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java
@@ -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")
@@ -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();
     }
   }
 
@@ -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();
@@ -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
@@ -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")
@@ -909,3 +913,4 @@ private static Status statusFromRemoteException(RemoteException e) {
     return Status.INTERNAL.withCause(e);
   }
 }
+