Skip to content

Commit 09fc908

Browse files
committed
looper: Require Send when adding fd event callbacks on ForeignLooper
`ForeignLooper` allows (re)registering callbacks on threads that are not the `ThreadLooper` (current) thread. Since these calls will be executed on that `ThreadLooper` thread when the user calls any of the `poll()` functions, these closures have to pass a thread boundary which requires `Send`.
1 parent 2d269e1 commit 09fc908

2 files changed

Lines changed: 77 additions & 4 deletions

File tree

ndk/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
- Move `MediaFormat` from `media::media_codec` to its own `media::media_format` module. (#442)
44
- media_format: Expose `MediaFormat::copy()` and `MediaFormat::clear()` from API level 29. (#449)
5+
- looper: Require `Send` marker when adding fd event callbacks on `ForeignLooper`. (#469)
56

67
# 0.8.0 (2023-10-15)
78

ndk/src/looper.rs

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,71 @@ impl ThreadLooper {
209209
)
210210
}
211211

212+
/// Adds a file descriptor to be polled, with a callback that is invoked when any of the
213+
/// [`FdEvent`]s described in `events` is triggered.
214+
///
215+
/// The callback receives the file descriptor it is associated with and a bitmask of the poll
216+
/// events that were triggered (typically [`FdEvent::INPUT`]). It should return [`true`] to
217+
/// continue receiving callbacks, or [`false`] to have the callback unregistered.
218+
///
219+
/// See also [the NDK
220+
/// docs](https://developer.android.com/ndk/reference/group/looper.html#alooper_addfd).
221+
///
222+
/// Note that this will leak a [`Box`] unless the callback returns [`false`] to unregister
223+
/// itself.
224+
///
225+
/// # Threading
226+
/// This function will be called on the current thread when this [`ThreadLooper`] is
227+
/// polled. A callback can also be registered from other threads via the equivalent
228+
/// [`ThreadLooper::add_fd_with_callback()`] function, which requires a [`Send`] bound.
229+
///
230+
/// # Safety
231+
/// The caller should guarantee that this file descriptor stays open until it is removed via
232+
/// [`remove_fd()`][Self::remove_fd()] or by returning [`false`] from the callback, and for
233+
/// however long the caller wishes to use this file descriptor inside and after the callback.
234+
#[doc(alias = "ALooper_addFd")]
235+
pub fn add_fd_with_callback<F: FnMut(BorrowedFd<'_>, FdEvent) -> bool>(
236+
&self,
237+
fd: BorrowedFd<'_>,
238+
events: FdEvent,
239+
callback: F,
240+
) -> Result<(), LooperError> {
241+
// let callback = unsafe { std::mem::transmute(callback) };
242+
// self.foreign.add_fd_with_callback(fd, events, callback)
243+
extern "C" fn cb_handler<F: FnMut(BorrowedFd<'_>, FdEvent) -> bool>(
244+
fd: RawFd,
245+
events: i32,
246+
data: *mut c_void,
247+
) -> i32 {
248+
abort_on_panic(|| unsafe {
249+
let mut cb = ManuallyDrop::new(Box::<F>::from_raw(data as *mut _));
250+
let events = FdEvent::from_bits_retain(
251+
events.try_into().expect("Unexpected sign bit in `events`"),
252+
);
253+
let keep_registered = cb(BorrowedFd::borrow_raw(fd), events);
254+
if !keep_registered {
255+
ManuallyDrop::into_inner(cb);
256+
}
257+
keep_registered as i32
258+
})
259+
}
260+
let data = Box::into_raw(Box::new(callback)) as *mut _;
261+
match unsafe {
262+
ffi::ALooper_addFd(
263+
self.as_foreign().ptr.as_ptr(),
264+
fd.as_raw_fd(),
265+
ffi::ALOOPER_POLL_CALLBACK,
266+
events.bits() as i32,
267+
Some(cb_handler::<F>),
268+
data,
269+
)
270+
} {
271+
1 => Ok(()),
272+
-1 => Err(LooperError),
273+
_ => unreachable!(),
274+
}
275+
}
276+
212277
/// Returns a reference to the [`ForeignLooper`] that is associated with the current thread.
213278
pub fn as_foreign(&self) -> &ForeignLooper {
214279
&self.foreign
@@ -311,9 +376,11 @@ impl ForeignLooper {
311376
}
312377
}
313378

314-
/// Adds a file descriptor to be polled, with a callback.
379+
/// Adds a file descriptor to be polled, with a callback that is invoked when any of the
380+
/// [`FdEvent`]s described in `events` is triggered.
315381
///
316-
/// The callback takes as an argument the file descriptor, and should return [`true`] to
382+
/// The callback receives the file descriptor it is associated with and a bitmask of the poll
383+
/// events that were triggered (typically [`FdEvent::INPUT`]). It should return [`true`] to
317384
/// continue receiving callbacks, or [`false`] to have the callback unregistered.
318385
///
319386
/// See also [the NDK
@@ -322,18 +389,23 @@ impl ForeignLooper {
322389
/// Note that this will leak a [`Box`] unless the callback returns [`false`] to unregister
323390
/// itself.
324391
///
392+
/// # Threading
393+
/// This function will be called on the looper thread where and when it is polled.
394+
/// For registering callbacks without [`Send`] requirement, call the equivalent
395+
/// [`ThreadLooper::add_fd_with_callback()`] function on the Looper thread.
396+
///
325397
/// # Safety
326398
/// The caller should guarantee that this file descriptor stays open until it is removed via
327399
/// [`remove_fd()`][Self::remove_fd()] or by returning [`false`] from the callback, and for
328400
/// however long the caller wishes to use this file descriptor inside and after the callback.
329401
#[doc(alias = "ALooper_addFd")]
330-
pub fn add_fd_with_callback<F: FnMut(BorrowedFd<'_>, FdEvent) -> bool>(
402+
pub fn add_fd_with_callback<F: FnMut(BorrowedFd<'_>, FdEvent) -> bool + Send>(
331403
&self,
332404
fd: BorrowedFd<'_>,
333405
events: FdEvent,
334406
callback: F,
335407
) -> Result<(), LooperError> {
336-
extern "C" fn cb_handler<F: FnMut(BorrowedFd<'_>, FdEvent) -> bool>(
408+
extern "C" fn cb_handler<F: FnMut(BorrowedFd<'_>, FdEvent) -> bool + Send>(
337409
fd: RawFd,
338410
events: i32,
339411
data: *mut c_void,

0 commit comments

Comments
 (0)