Fix split BR_TRANSACTION_COMPLETE/BR_REPLY and EINTR handling

Two binder protocol bugs fixed:

1. The kernel can send BR_TRANSACTION_COMPLETE and BR_REPLY in
   separate ioctl reads. When the target service hasn't responded
   by the time the first ioctl returns, we only get TC. Previously
   we returned an empty parcel; now we issue another read ioctl to
   wait for BR_REPLY. This fixes ~17% failure rate under rapid
   open/close cycles (50 iterations: 250/250 pass, was ~40/50).

2. EINTR (interrupted system call) on any ioctl is now retried
   instead of returning an error.

Author: xaionaro@dx.center

kernelbinder/driver.go

@@ -222,23 +222,67 @@ func (d *Driver) Transact(
 		readBuffer:  uint64(uintptr(unsafe.Pointer(&readBuf[0]))),
 	}
 
-	_, _, errno := unix.Syscall(
-		unix.SYS_IOCTL,
-		uintptr(d.fd),
-		binderWriteReadIoctl,
-		uintptr(unsafe.Pointer(&bwr)),
-	)
-	if errno != 0 {
-		return nil, &aidlerrors.BinderError{Op: "ioctl(BINDER_WRITE_READ)", Err: errno}
+	for {
+		_, _, errno := unix.Syscall(
+			unix.SYS_IOCTL,
+			uintptr(d.fd),
+			binderWriteReadIoctl,
+			uintptr(unsafe.Pointer(&bwr)),
+		)
+		switch errno {
+		case 0:
+			// Success — proceed to parse.
+		case unix.EINTR:
+			// Retry on interrupted system call.
+			// Reset write buffer (already consumed) but keep read buffer.
+			bwr.writeSize = 0
+			continue
+		default:
+			return nil, &aidlerrors.BinderError{Op: "ioctl(BINDER_WRITE_READ)", Err: errno}
+		}
+		break
 	}
 
-	// Parse the read buffer for BR codes.
-	reply, err := d.parseReadBuffer(ctx, readBuf[:bwr.readConsumed])
-	if err != nil {
-		return nil, err
-	}
+	// Parse the read buffer for BR codes. The kernel may split
+	// BR_TRANSACTION_COMPLETE and BR_REPLY across separate ioctl reads —
+	// if we got TC but no reply yet, read again to wait for BR_REPLY.
+	isOneway := flags&binder.FlagOneway != 0
+	for {
+		reply, err := d.parseReadBuffer(ctx, readBuf[:bwr.readConsumed])
+		if err != nil {
+			return nil, err
+		}
 
-	return reply, nil
+		// If we got a non-empty reply, or this is a oneway transaction
+		// (which never gets BR_REPLY), return immediately.
+		if reply.Len() > 0 || isOneway {
+			return reply, nil
+		}
+
+		// Empty reply on a non-oneway transaction means the kernel sent
+		// BR_TRANSACTION_COMPLETE but BR_REPLY hasn't arrived yet.
+		// Issue a read-only ioctl to wait for it.
+		logger.Debugf(ctx, "Transact: got BR_TRANSACTION_COMPLETE without BR_REPLY, reading again")
+		bwr.writeSize = 0
+		bwr.writeConsumed = 0
+		bwr.readConsumed = 0
+		for {
+			_, _, errno := unix.Syscall(
+				unix.SYS_IOCTL,
+				uintptr(d.fd),
+				binderWriteReadIoctl,
+				uintptr(unsafe.Pointer(&bwr)),
+			)
+			switch errno {
+			case 0:
+			case unix.EINTR:
+				continue
+			default:
+				return nil, &aidlerrors.BinderError{Op: "ioctl(BINDER_WRITE_READ/read)", Err: errno}
+			}
+			break
+		}
+	}
 }
 
 // parseReadBuffer processes BR_* codes from the read buffer and returns the reply parcel.
@@ -496,17 +540,24 @@ func (d *Driver) writeCommand(
 		writeBuffer: uint64(uintptr(unsafe.Pointer(&writeBuf[0]))),
 	}
 
-	_, _, errno := unix.Syscall(
-		unix.SYS_IOCTL,
-		uintptr(d.fd),
-		binderWriteReadIoctl,
-		uintptr(unsafe.Pointer(&bwr)),
-	)
-	if errno != 0 {
-		return &aidlerrors.BinderError{Op: "ioctl(BINDER_WRITE_READ)", Err: errno}
+	for {
+		_, _, errno := unix.Syscall(
+			unix.SYS_IOCTL,
+			uintptr(d.fd),
+			binderWriteReadIoctl,
+			uintptr(unsafe.Pointer(&bwr)),
+		)
+		switch errno {
+		case 0:
+			return nil
+		case unix.EINTR:
+			// Retry on interrupted system call — the command was not
+			// processed and must be resent.
+			continue
+		default:
+			return &aidlerrors.BinderError{Op: "ioctl(BINDER_WRITE_READ)", Err: errno}
+		}
 	}
-
-	return nil
 }
 
 // copyStructToBytes copies a struct's raw memory into a byte slice.

tests/e2e/aidlcli_test.go

@@ -191,7 +191,6 @@ func logAndExit(msg string) {
 // Arguments are joined into a single shell command string to preserve
 // empty/quoted values across the adb shell boundary.
 func runAidlcli(args ...string) (string, string, error) {
-	// Build a single shell command string to avoid adb shell arg splitting issues.
 	parts := make([]string, 0, len(args)+3)
 	parts = append(parts, deviceBinary, "--format", "json")
 	for _, a := range args {