Skip to content

Commit

Permalink
ndk/looper: Add remove_fd() method (#416)
Browse files Browse the repository at this point in the history
We've previously only been able to unregister file descriptors by using
a callback and returning `false` from it to be unregistered.  Add the
missing method that allows users to unregister their file descriptors
from the looper for any reason.

There has always been a "concern" about leaking the `Box` inside
`add_fd_with_callback()`, but this is already very easy by never
returning `false` from the callback or registering a different callback
/ event on the same `fd`, so this new function doesn't really expose
that.
  • Loading branch information
MarijnS95 authored Aug 10, 2023
1 parent abe5fca commit a3c34f7
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
1 change: 1 addition & 0 deletions ndk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- hardware_buffer_format: Add `YCbCr_P010` and `R8_UNORM` variants. (#405)
- **Breaking:** hardware_buffer_format: Add catch-all variant. (#407)
- Add panic guards to callbacks. (#412)
- looper: Add `remove_fd()` to unregister events/callbacks for a file descriptor. (#416)

# 0.7.0 (2022-07-24)

Expand Down
33 changes: 31 additions & 2 deletions ndk/src/looper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,14 @@ impl ForeignLooper {

/// Adds a file descriptor to be polled, with a callback.
///
/// The callback takes as an argument the file descriptor, and should return true to continue
/// receiving callbacks, or false to have the callback unregistered.
/// The callback takes as an argument the file descriptor, and should return [`true`] to
/// continue receiving callbacks, or [`false`] to have the callback unregistered.
///
/// See also [the NDK
/// docs](https://developer.android.com/ndk/reference/group/looper.html#alooper_addfd).
///
/// Note that this will leak a [`Box`] unless the callback returns [`false`] to unregister
/// itself.
pub fn add_fd_with_callback<F: FnMut(RawFd) -> bool>(
&self,
fd: RawFd,
Expand Down Expand Up @@ -307,4 +310,30 @@ impl ForeignLooper {
_ => unreachable!(),
}
}

/// Removes a previously added file descriptor from the looper.
///
/// Returns [`true`] if the file descriptor was removed, [`false`] if it was not previously
/// registered.
///
/// # Safety
/// When this method returns, it is safe to close the file descriptor since the looper will no
/// longer have a reference to it. However, it is possible for the callback to already be
/// running or for it to run one last time if the file descriptor was already signalled.
/// Calling code is responsible for ensuring that this case is safely handled. For example, if
/// the callback takes care of removing itself during its own execution either by returning `0`
/// or by calling this method, then it can be guaranteed to not be invoked again at any later
/// time unless registered anew.
///
/// Note that unregistering a file descriptor with callback will leak a [`Box`] created in
/// [`add_fd_with_callback()`][Self::add_fd_with_callback()]. Consider returning [`false`]
/// from the callback instead to drop it.
pub fn remove_fd(&self, fd: RawFd) -> Result<bool, LooperError> {
match unsafe { ffi::ALooper_removeFd(self.ptr.as_ptr(), fd) } {
1 => Ok(true),
0 => Ok(false),
-1 => Err(LooperError),
_ => unreachable!(),
}
}
}

0 comments on commit a3c34f7

Please sign in to comment.