Skip to content

Commit c98a491

Browse files
xaionaro@dx.centerxaionaro@dx.center
authored andcommitted
fix: Close() deadlock when read loop holds fdMu during blocking ioctl
The read loop holds fdMu.RLock across a blocking BINDER_WRITE_READ ioctl. Close() tried to take fdMu.Lock (write) before closing the fd, creating a deadlock: the ioctl won't return until the fd is closed, but Close() can't close the fd until the ioctl releases RLock. Fix: close the raw fd first to interrupt blocked ioctls with EBADF, then take fdMu.Lock to invalidate the fd field. No new Transact calls can start because d.closed is already set. Also fix camera_connect example: replace os.Exit(0) with return so deferred drv.Close runs, and add Disconnect call on success path.
1 parent 627a8e7 commit c98a491

2 files changed

Lines changed: 30 additions & 12 deletions

File tree

examples/camera_connect/main.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,16 @@ func main() {
112112
deviceUser, err := cameraProxy.ConnectDevice(ctx, callback, "0")
113113
if err != nil {
114114
fmt.Fprintf(os.Stderr, "ConnectDevice error (expected): %v\n", err)
115-
os.Exit(0)
115+
// Let deferred drv.Close run for clean shutdown.
116+
return
116117
}
117118

118119
fmt.Printf("ConnectDevice succeeded: %v\n", deviceUser)
120+
121+
// Disconnect the camera device before closing the binder driver.
122+
// This tells the camera service to release the callback, so the
123+
// binder driver's read loop is not stuck waiting for transactions.
124+
if err := deviceUser.Disconnect(ctx); err != nil {
125+
fmt.Fprintf(os.Stderr, "Disconnect: %v\n", err)
126+
}
119127
}

kernelbinder/driver.go

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -224,12 +224,28 @@ func (d *Driver) Close(
224224
}
225225
}
226226

227-
// Take fdMu write lock to wait for all in-flight ioctls (which
228-
// hold RLock) to finish, then invalidate fd/mapped so no new
229-
// operations can use them. The actual munmap and close happen
230-
// outside both locks using local copies.
231-
d.fdMu.Lock()
227+
// Close the raw fd FIRST to interrupt any blocked ioctl (e.g. the
228+
// read loop's BINDER_WRITE_READ). The read loop holds fdMu.RLock
229+
// across a blocking ioctl, so taking fdMu.Lock here would deadlock
230+
// if the ioctl never returns. Closing the fd wakes the ioctl with
231+
// EBADF, allowing it to release the RLock.
232+
//
233+
// No new Transact calls can start because d.closed is already true
234+
// (checked under d.mu). The read loop will see EBADF and exit.
235+
d.fdMu.RLock()
232236
fd := d.fd
237+
d.fdMu.RUnlock()
238+
239+
if fd >= 0 {
240+
if err := unix.Close(fd); err != nil {
241+
errs = append(errs, &aidlerrors.BinderError{Op: "close", Err: err})
242+
}
243+
}
244+
245+
// Now take the write lock to wait for all in-flight ioctls to
246+
// finish (they will see EBADF and return promptly), then
247+
// invalidate fd/mapped so no stale accesses occur.
248+
d.fdMu.Lock()
233249
mapped := d.mapped
234250
d.fd = -1
235251
d.mapped = nil
@@ -241,12 +257,6 @@ func (d *Driver) Close(
241257
}
242258
}
243259

244-
if fd >= 0 {
245-
if err := unix.Close(fd); err != nil {
246-
errs = append(errs, &aidlerrors.BinderError{Op: "close", Err: err})
247-
}
248-
}
249-
250260
// Wait for the read loop goroutine to exit after closing the fd.
251261
// The read loop detects the closed fd via EBADF and returns.
252262
// Only wait if the read loop was actually started; otherwise

0 commit comments

Comments
 (0)