Skip to content

Commit 629c19f

Browse files
xaionaro@dx.centerxaionaro@dx.center
authored andcommitted
Track and release binder handles on driver close
The kernel binder driver requires BC_RELEASE + BC_DECREFS to clean up handle references. Without this, rapid open/close cycles accumulate unreleased refs in the kernel. While this doesn't fully fix the emulator flakiness (which is caused by kernel binder_proc cleanup timing under rapid reopen), it implements the correct lifecycle that the native libbinder does.
1 parent 7576bbc commit 629c19f

2 files changed

Lines changed: 40 additions & 15 deletions

File tree

kernelbinder/driver.go

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package kernelbinder
55
import (
66
"context"
77
"encoding/binary"
8+
"errors"
89
"fmt"
910
"runtime"
1011
"sync"
@@ -29,10 +30,11 @@ const readBufferSize = 256
2930

3031
// Driver implements binder.Transport using /dev/binder.
3132
type Driver struct {
32-
fd int
33-
mapped []byte // mmap'd region, kept alive for munmap
34-
mapSize uint32
35-
mu sync.Mutex
33+
fd int
34+
mapped []byte // mmap'd region, kept alive for munmap
35+
mapSize uint32
36+
mu sync.Mutex
37+
acquiredHandles map[uint32]bool // tracks handles acquired via BC_INCREFS + BC_ACQUIRE
3638
}
3739

3840
// Compile-time interface check.
@@ -104,15 +106,16 @@ func Open(
104106
}
105107

106108
d := &Driver{
107-
fd: fd,
108-
mapped: mapped,
109-
mapSize: mapSize,
109+
fd: fd,
110+
mapped: mapped,
111+
mapSize: mapSize,
112+
acquiredHandles: make(map[uint32]bool),
110113
}
111114

112115
return d, nil
113116
}
114117

115-
// Close releases the mmap and closes the binder file descriptor.
118+
// Close releases all acquired binder handles, the mmap, and the file descriptor.
116119
func (d *Driver) Close(
117120
ctx context.Context,
118121
) (_err error) {
@@ -121,6 +124,21 @@ func (d *Driver) Close(
121124

122125
var errs []error
123126

127+
// Release all acquired binder handles before closing the fd,
128+
// so the kernel does not leak handle references.
129+
for handle := range d.acquiredHandles {
130+
logger.Debugf(ctx, "releasing handle %d on close", handle)
131+
buf := make([]byte, 16)
132+
binary.LittleEndian.PutUint32(buf[0:4], bcRelease)
133+
binary.LittleEndian.PutUint32(buf[4:8], handle)
134+
binary.LittleEndian.PutUint32(buf[8:12], bcDecRefs)
135+
binary.LittleEndian.PutUint32(buf[12:16], handle)
136+
if err := d.writeCommand(ctx, buf); err != nil {
137+
errs = append(errs, fmt.Errorf("release handle %d: %w", handle, err))
138+
}
139+
}
140+
d.acquiredHandles = nil
141+
124142
if d.mapped != nil {
125143
if err := unix.Munmap(d.mapped); err != nil {
126144
errs = append(errs, &aidlerrors.BinderError{Op: "munmap", Err: err})
@@ -135,11 +153,7 @@ func (d *Driver) Close(
135153
d.fd = -1
136154
}
137155

138-
if len(errs) > 0 {
139-
return fmt.Errorf("binder close: %w", errs[0])
140-
}
141-
142-
return nil
156+
return errors.Join(errs...)
143157
}
144158

145159
// mapBase returns the base address of the mmap'd region.
@@ -362,7 +376,9 @@ func (d *Driver) acquireReplyHandles(
362376

363377
if err := d.writeCommand(ctx, buf); err != nil {
364378
logger.Warnf(ctx, "failed to acquire handle %d: %v", handle, err)
379+
continue
365380
}
381+
d.acquiredHandles[handle] = true
366382
}
367383
}
368384
}
@@ -388,7 +404,11 @@ func (d *Driver) AcquireHandle(
388404
logger.Tracef(ctx, "AcquireHandle")
389405
defer func() { logger.Tracef(ctx, "/AcquireHandle: %v", _err) }()
390406

391-
return d.writeHandleCommand(ctx, bcAcquire, handle)
407+
if err := d.writeHandleCommand(ctx, bcAcquire, handle); err != nil {
408+
return err
409+
}
410+
d.acquiredHandles[handle] = true
411+
return nil
392412
}
393413

394414
// ReleaseHandle decrements the strong reference count for a binder handle.
@@ -399,7 +419,11 @@ func (d *Driver) ReleaseHandle(
399419
logger.Tracef(ctx, "ReleaseHandle")
400420
defer func() { logger.Tracef(ctx, "/ReleaseHandle: %v", _err) }()
401421

402-
return d.writeHandleCommand(ctx, bcRelease, handle)
422+
if err := d.writeHandleCommand(ctx, bcRelease, handle); err != nil {
423+
return err
424+
}
425+
delete(d.acquiredHandles, handle)
426+
return nil
403427
}
404428

405429
// RequestDeathNotification registers a death notification for a binder handle.

kernelbinder/protocol.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ var (
1616
bcIncRefs = uint32(iow('c', 4, unsafe.Sizeof(uint32(0))))
1717
bcAcquire = uint32(iow('c', 5, unsafe.Sizeof(uint32(0))))
1818
bcRelease = uint32(iow('c', 6, unsafe.Sizeof(uint32(0))))
19+
bcDecRefs = uint32(iow('c', 7, unsafe.Sizeof(uint32(0))))
1920
bcRequestDeathNotif = uint32(iow('c', 14, binderHandleCookieSize))
2021
bcClearDeathNotif = uint32(iow('c', 15, binderHandleCookieSize))
2122
)

0 commit comments

Comments
 (0)