binder: Rationalize @ThreadSafe-ty of BinderTransport. (#12130)

- Use @BinderThread to document restrictions on methods and certain fields.
- Make TransactionHandler non-public since only Android should call it.
- Replace an unnecessary AtomicLong with a plain old long.
This commit is contained in:
John Cormie 2025-06-05 19:07:59 -07:00 committed by GitHub
parent 4cd7881086
commit c206428749
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 17 additions and 13 deletions

View File

@ -29,6 +29,7 @@ import android.os.Parcel;
import android.os.Process;
import android.os.RemoteException;
import android.os.TransactionTooLargeException;
import androidx.annotation.BinderThread;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Ticker;
import com.google.common.base.Verify;
@ -105,8 +106,7 @@ import javax.annotation.concurrent.ThreadSafe;
* https://github.com/grpc/proposal/blob/master/L73-java-binderchannel/wireformat.md
*/
@ThreadSafe
public abstract class BinderTransport
implements LeakSafeOneWayBinder.TransactionHandler, IBinder.DeathRecipient {
public abstract class BinderTransport implements IBinder.DeathRecipient {
private static final Logger logger = Logger.getLogger(BinderTransport.class.getName());
@ -210,9 +210,11 @@ public abstract class BinderTransport
private final FlowController flowController;
/** The number of incoming bytes we've received. */
private final AtomicLong numIncomingBytes;
// Only read/written on @BinderThread.
private long numIncomingBytes;
/** The number of incoming bytes we've told our peer we've received. */
// Only read/written on @BinderThread.
private long acknowledgedIncomingBytes;
private BinderTransport(
@ -225,10 +227,9 @@ public abstract class BinderTransport
this.attributes = attributes;
this.logId = logId;
scheduledExecutorService = executorServicePool.getObject();
incomingBinder = new LeakSafeOneWayBinder(this);
incomingBinder = new LeakSafeOneWayBinder(this::handleTransaction);
ongoingCalls = new ConcurrentHashMap<>();
flowController = new FlowController(TRANSACTION_BYTES_WINDOW);
numIncomingBytes = new AtomicLong();
}
// Override in child class.
@ -423,8 +424,9 @@ public abstract class BinderTransport
}
}
@Override
public final boolean handleTransaction(int code, Parcel parcel) {
@BinderThread
@VisibleForTesting
final boolean handleTransaction(int code, Parcel parcel) {
try {
return handleTransactionInternal(code, parcel);
} catch (RuntimeException e) {
@ -440,6 +442,7 @@ public abstract class BinderTransport
}
}
@BinderThread
private boolean handleTransactionInternal(int code, Parcel parcel) {
if (code < FIRST_CALL_ID) {
synchronized (this) {
@ -483,11 +486,12 @@ public abstract class BinderTransport
if (inbound != null) {
inbound.handleTransaction(parcel);
}
long nib = numIncomingBytes.addAndGet(size);
if ((nib - acknowledgedIncomingBytes) > TRANSACTION_BYTES_WINDOW_FORCE_ACK) {
numIncomingBytes += size;
if ((numIncomingBytes - acknowledgedIncomingBytes) > TRANSACTION_BYTES_WINDOW_FORCE_ACK) {
synchronized (this) {
sendAcknowledgeBytes(checkNotNull(outgoingBinder));
sendAcknowledgeBytes(checkNotNull(outgoingBinder), numIncomingBytes);
}
acknowledgedIncomingBytes = numIncomingBytes;
}
return true;
}
@ -519,10 +523,8 @@ public abstract class BinderTransport
protected void handlePingResponse(Parcel parcel) {}
@GuardedBy("this")
private void sendAcknowledgeBytes(OneWayBinderProxy iBinder) {
private void sendAcknowledgeBytes(OneWayBinderProxy iBinder, long n) {
// Send a transaction to acknowledge reception of incoming data.
long n = numIncomingBytes.get();
acknowledgedIncomingBytes = n;
try (ParcelHolder parcel = ParcelHolder.obtain()) {
parcel.get().writeLong(n);
iBinder.transact(ACKNOWLEDGE_BYTES, parcel);

View File

@ -19,6 +19,7 @@ package io.grpc.binder.internal;
import android.os.Binder;
import android.os.IBinder;
import android.os.Parcel;
import androidx.annotation.BinderThread;
import io.grpc.Internal;
import java.util.logging.Level;
import java.util.logging.Logger;
@ -58,6 +59,7 @@ public final class LeakSafeOneWayBinder extends Binder {
* @return the value to return from {@link Binder#onTransact}. NB: "oneway" semantics mean this
* result will not delivered to the caller of {@link IBinder#transact}
*/
@BinderThread
boolean handleTransaction(int code, Parcel data);
}