Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(ffi): introduce AsyncRuntimeDropped helper #4168

Merged
merged 1 commit into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 3 additions & 17 deletions bindings/matrix-sdk-ffi/src/client.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::{
collections::HashMap,
fmt::Debug,
mem::ManuallyDrop,
path::Path,
sync::{Arc, RwLock},
};
Expand Down Expand Up @@ -79,6 +78,7 @@ use crate::{
ruma::AuthData,
sync_service::{SyncService, SyncServiceBuilder},
task_handle::TaskHandle,
utils::AsyncRuntimeDropped,
ClientError,
};

Expand Down Expand Up @@ -184,26 +184,12 @@ impl From<matrix_sdk::TransmissionProgress> for TransmissionProgress {

#[derive(uniffi::Object)]
pub struct Client {
pub(crate) inner: ManuallyDrop<MatrixClient>,
pub(crate) inner: AsyncRuntimeDropped<MatrixClient>,
delegate: RwLock<Option<Arc<dyn ClientDelegate>>>,
session_verification_controller:
Arc<tokio::sync::RwLock<Option<SessionVerificationController>>>,
}

impl Drop for Client {
fn drop(&mut self) {
// Dropping the inner OlmMachine must happen within a tokio context
// because deadpool drops sqlite connections in the DB pool on tokio's
// blocking threadpool to avoid blocking async worker threads.
let _guard = RUNTIME.enter();
// SAFETY: self.inner is never used again, which is the only requirement
// for ManuallyDrop::drop to be used safely.
unsafe {
ManuallyDrop::drop(&mut self.inner);
}
}
}

impl Client {
pub async fn new(
sdk_client: MatrixClient,
Expand All @@ -224,7 +210,7 @@ impl Client {
});

let client = Client {
inner: ManuallyDrop::new(sdk_client),
inner: AsyncRuntimeDropped::new(sdk_client),
delegate: RwLock::new(None),
session_verification_controller,
};
Expand Down
11 changes: 3 additions & 8 deletions bindings/matrix-sdk-ffi/src/room_list.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
#![allow(deprecated)]

use std::{
fmt::Debug,
mem::{ManuallyDrop, MaybeUninit},
ptr::addr_of_mut,
sync::Arc,
time::Duration,
};
use std::{fmt::Debug, mem::MaybeUninit, ptr::addr_of_mut, sync::Arc, time::Duration};

use eyeball_im::VectorDiff;
use futures_util::{pin_mut, StreamExt, TryFutureExt};
Expand Down Expand Up @@ -34,6 +28,7 @@ use crate::{
room_preview::RoomPreview,
timeline::{EventTimelineItem, Timeline},
timeline_event_filter::TimelineEventTypeFilter,
utils::AsyncRuntimeDropped,
TaskHandle, RUNTIME,
};

Expand Down Expand Up @@ -635,7 +630,7 @@ impl RoomListItem {

let room_preview = client.get_room_preview(&room_or_alias_id, server_names).await?;

Ok(Arc::new(RoomPreview::new(ManuallyDrop::new(client), room_preview)))
Ok(Arc::new(RoomPreview::new(AsyncRuntimeDropped::new(client), room_preview)))
}

/// Build a full `Room` FFI object, filling its associated timeline.
Expand Down
23 changes: 3 additions & 20 deletions bindings/matrix-sdk-ffi/src/room_preview.rs
Original file line number Diff line number Diff line change
@@ -1,33 +1,16 @@
use std::mem::ManuallyDrop;

use anyhow::Context as _;
use async_compat::TOKIO1 as RUNTIME;
use matrix_sdk::{room_preview::RoomPreview as SdkRoomPreview, Client};
use ruma::space::SpaceRoomJoinRule;
use tracing::warn;

use crate::{client::JoinRule, error::ClientError, room::Membership};
use crate::{client::JoinRule, error::ClientError, room::Membership, utils::AsyncRuntimeDropped};

/// A room preview for a room. It's intended to be used to represent rooms that
/// aren't joined yet.
#[derive(uniffi::Object)]
pub struct RoomPreview {
inner: SdkRoomPreview,
client: ManuallyDrop<Client>,
}

impl Drop for RoomPreview {
fn drop(&mut self) {
// Dropping the inner OlmMachine must happen within a tokio context
// because deadpool drops sqlite connections in the DB pool on tokio's
// blocking threadpool to avoid blocking async worker threads.
let _guard = RUNTIME.enter();
// SAFETY: self.client is never used again, which is the only requirement
// for ManuallyDrop::drop to be used safely.
unsafe {
ManuallyDrop::drop(&mut self.client);
}
}
client: AsyncRuntimeDropped<Client>,
}

#[matrix_sdk_ffi_macros::export]
Expand Down Expand Up @@ -65,7 +48,7 @@ impl RoomPreview {
}

impl RoomPreview {
pub(crate) fn new(client: ManuallyDrop<Client>, inner: SdkRoomPreview) -> Self {
pub(crate) fn new(client: AsyncRuntimeDropped<Client>, inner: SdkRoomPreview) -> Self {
Self { client, inner }
}
}
Expand Down
45 changes: 45 additions & 0 deletions bindings/matrix-sdk-ffi/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::{mem::ManuallyDrop, ops::Deref};

use async_compat::TOKIO1 as RUNTIME;
use ruma::UInt;
use tracing::warn;

Expand All @@ -21,3 +24,45 @@ pub(crate) fn u64_to_uint(u: u64) -> UInt {
UInt::MAX
})
}

/// Tiny wrappers for data types that must be dropped in the context of an async
/// runtime.
///
/// This is useful whenever such a data type may transitively call some
/// runtime's `block_on` function in their `Drop` impl (since we lack async drop
/// at the moment), like done in some `deadpool` drop impls.
pub(crate) struct AsyncRuntimeDropped<T>(ManuallyDrop<T>);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please!


impl<T> AsyncRuntimeDropped<T> {
/// Create a new wrapper for this type that will be dropped under an async
/// runtime.
pub fn new(val: T) -> Self {
Self(ManuallyDrop::new(val))
}
}

impl<T> Drop for AsyncRuntimeDropped<T> {
fn drop(&mut self) {
let _guard = RUNTIME.enter();
// SAFETY: self.inner is never used again, which is the only requirement
// for ManuallyDrop::drop to be used safely.
unsafe {
ManuallyDrop::drop(&mut self.0);
}
}
}

// What is an `AsyncRuntimeDropped<T>`, if not a `T` in disguise?
impl<T> Deref for AsyncRuntimeDropped<T> {
type Target = T;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<T: Clone> Clone for AsyncRuntimeDropped<T> {
fn clone(&self) -> Self {
Self(self.0.clone())
}
}
Loading