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

server: allow accessing resource in Dispatch::destroyed #654

Merged
merged 1 commit into from
Sep 2, 2023
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
1 change: 1 addition & 0 deletions wayland-backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
needs to be dispatched using `Backend::dispatch_inner_queue()`, instead of trying to dispatch
it by itself. This can only happen when using the `sys` backend, and allows the crate to
behave properly when multiple threads try to read the socket using the libwayland API.
- server: `ObjectData::destroyed` function signature has changed to pass the `Handle` and `self` as `Arc<Self>`.

#### Additions

Expand Down
9 changes: 5 additions & 4 deletions wayland-backend/src/rs/server_impl/common_poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
) -> std::io::Result<usize> {
let ret = self.dispatch_events_for(data, client_id);
let cleanup = self.state.lock().unwrap().cleanup();
cleanup(data);
cleanup(&self.handle(), data);

Check warning on line 77 in wayland-backend/src/rs/server_impl/common_poll.rs

View check run for this annotation

Codecov / codecov/patch

wayland-backend/src/rs/server_impl/common_poll.rs#L77

Added line #L77 was not covered by tests
ret
}

Expand All @@ -98,7 +98,7 @@
}
}
let cleanup = self.state.lock().unwrap().cleanup();
cleanup(data);
cleanup(&self.handle(), data);
}

Ok(dispatched)
Expand Down Expand Up @@ -137,7 +137,7 @@
}
}
let cleanup = self.state.lock().unwrap().cleanup();
cleanup(data);
cleanup(&self.handle(), data);
}

Ok(dispatched)
Expand Down Expand Up @@ -230,7 +230,8 @@
},
);
if is_destructor {
object.data.user_data.destroyed(
object.data.user_data.clone().destroyed(
&handle.clone(),
data,
ClientId { id: client_id.clone() },
ObjectId { id: object_id.clone() },
Expand Down
11 changes: 8 additions & 3 deletions wayland-backend/src/rs/server_impl/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,19 @@ impl<D> State<D> {
}
}

pub(crate) fn cleanup(&mut self) -> impl FnOnce(&mut D) {
pub(crate) fn cleanup<'a>(&mut self) -> impl FnOnce(&super::Handle, &mut D) + 'a {
let dead_clients = self.clients.cleanup(&mut self.pending_destructors);
self.registry.cleanup(&dead_clients);
// return a closure that will do the cleanup once invoked
let pending_destructors = std::mem::take(&mut self.pending_destructors);
move |data| {
move |handle, data| {
for (object_data, client_id, object_id) in pending_destructors {
object_data.destroyed(data, ClientId { id: client_id }, ObjectId { id: object_id });
object_data.clone().destroyed(
handle,
data,
ClientId { id: client_id },
ObjectId { id: object_id },
);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion wayland-backend/src/rs/server_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl<D> ObjectData<D> for UninitObjectData {
}

#[cfg_attr(coverage, no_coverage)]
fn destroyed(&self, _: &mut D, _: ClientId, _: ObjectId) {}
fn destroyed(self: Arc<Self>, _: &Handle, _: &mut D, _: ClientId, _: ObjectId) {}

#[cfg_attr(coverage, no_coverage)]
fn debug(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Expand Down
17 changes: 15 additions & 2 deletions wayland-backend/src/server_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,13 @@ pub trait ObjectData<D>: downcast_rs::DowncastSync {
msg: Message<ObjectId, OwnedFd>,
) -> Option<Arc<dyn ObjectData<D>>>;
/// Notification that the object has been destroyed and is no longer active
fn destroyed(&self, data: &mut D, client_id: ClientId, object_id: ObjectId);
fn destroyed(
self: Arc<Self>,
handle: &Handle,
data: &mut D,
client_id: ClientId,
object_id: ObjectId,
);
/// Helper for forwarding a Debug implementation of your `ObjectData` type
///
/// By default will just print `ObjectData { ... }`
Expand Down Expand Up @@ -585,5 +591,12 @@ impl<D> ObjectData<D> for DumbObjectData {
}

#[cfg_attr(coverage, no_coverage)]
fn destroyed(&self, _: &mut D, _client_id: ClientId, _object_id: ObjectId) {}
fn destroyed(
self: Arc<Self>,
_handle: &Handle,
_: &mut D,
_client_id: ClientId,
_object_id: ObjectId,
) {
}
}
14 changes: 10 additions & 4 deletions wayland-backend/src/sys/server_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,8 @@
let pending_destructors =
std::mem::take(&mut self.state.lock().unwrap().pending_destructors);
for (object, client_id, object_id) in pending_destructors {
object.destroyed(data, client_id, object_id);
let handle = self.handle();
object.clone().destroyed(&handle, data, client_id, object_id);

Check warning on line 417 in wayland-backend/src/sys/server_impl/mod.rs

View check run for this annotation

Codecov / codecov/patch

wayland-backend/src/sys/server_impl/mod.rs#L416-L417

Added lines #L416 - L417 were not covered by tests
}

if ret < 0 {
Expand Down Expand Up @@ -1601,10 +1602,15 @@
let object_id =
InnerObjectId { interface: udata.interface, ptr: resource, alive: udata.alive.clone(), id };
if HANDLE.is_set() {
HANDLE.with(|&(_, data_ptr)| {
HANDLE.with(|&(ref state_arc, data_ptr)| {
// Safety: the data pointer have been set by outside code and are valid
let data = unsafe { &mut *(data_ptr as *mut D) };
udata.data.destroyed(data, ClientId { id: client_id }, ObjectId { id: object_id });
udata.data.destroyed(
&Handle { handle: InnerHandle { state: state_arc.clone() } },
data,
ClientId { id: client_id },
ObjectId { id: object_id },
);
});
} else {
PENDING_DESTRUCTORS.with(|&pending_ptr| {
Expand Down Expand Up @@ -1639,7 +1645,7 @@
}

#[cfg_attr(coverage, no_coverage)]
fn destroyed(&self, _: &mut D, _: ClientId, _: ObjectId) {}
fn destroyed(self: Arc<Self>, _: &Handle, _: &mut D, _: ClientId, _: ObjectId) {}

#[cfg_attr(coverage, no_coverage)]
fn debug(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Expand Down
3 changes: 2 additions & 1 deletion wayland-backend/src/test/destructors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ macro_rules! impl_server_objectdata {
}

fn destroyed(
&self,
self: Arc<Self>,
_: &$server_backend::Handle,
_: &mut (),
_: $server_backend::ClientId,
_: $server_backend::ObjectId,
Expand Down
9 changes: 8 additions & 1 deletion wayland-backend/src/test/many_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,14 @@ macro_rules! serverdata_impls {
None
}

fn destroyed(&self, _: &mut (), _: $server_backend::ClientId, _: $server_backend::ObjectId) {}
fn destroyed(
self: Arc<Self>,
_: &$server_backend::Handle,
_: &mut (),
_: $server_backend::ClientId,
_: $server_backend::ObjectId
) {
}
}

impl $server_backend::GlobalHandler<()> for ServerData {
Expand Down
18 changes: 16 additions & 2 deletions wayland-backend/src/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,14 @@ impl<D> server_rs::ObjectData<D> for DoNothingData {
None
}

fn destroyed(&self, _: &mut D, _: server_rs::ClientId, _: server_rs::ObjectId) {}
fn destroyed(
self: Arc<Self>,
_handle: &server_rs::Handle,
_: &mut D,
_: server_rs::ClientId,
_: server_rs::ObjectId,
) {
}
}

impl<D> server_sys::ObjectData<D> for DoNothingData {
Expand All @@ -156,7 +163,14 @@ impl<D> server_sys::ObjectData<D> for DoNothingData {
None
}

fn destroyed(&self, _: &mut D, _: server_sys::ClientId, _: server_sys::ObjectId) {}
fn destroyed(
self: Arc<Self>,
_handle: &server_sys::Handle,
_: &mut D,
_: server_sys::ClientId,
_: server_sys::ObjectId,
) {
}
}

// Client Object Data
Expand Down
9 changes: 8 additions & 1 deletion wayland-backend/src/test/object_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,14 @@ macro_rules! impl_server_objectdata {
None
}

fn destroyed(&self, _: &mut (), _: $server_backend::ClientId, _: $server_backend::ObjectId) {}
fn destroyed(
self: Arc<Self>,
_: &$server_backend::Handle,
_: &mut (),
_: $server_backend::ClientId,
_: $server_backend::ObjectId
) {
}
}

impl $server_backend::GlobalHandler<()> for ServerData {
Expand Down
18 changes: 16 additions & 2 deletions wayland-backend/src/test/protocol_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,14 @@ impl<D> server_rs::ObjectData<D> for ProtocolErrorServerData {
None
}

fn destroyed(&self, _: &mut D, _: server_rs::ClientId, _: server_rs::ObjectId) {}
fn destroyed(
self: Arc<Self>,
_handle: &server_rs::Handle,
_: &mut D,
_: server_rs::ClientId,
_: server_rs::ObjectId,
) {
}
}

impl<D> server_sys::ObjectData<D> for ProtocolErrorServerData {
Expand All @@ -238,7 +245,14 @@ impl<D> server_sys::ObjectData<D> for ProtocolErrorServerData {
None
}

fn destroyed(&self, _: &mut D, _: server_sys::ClientId, _: server_sys::ObjectId) {}
fn destroyed(
self: Arc<Self>,
_handle: &server_sys::Handle,
_: &mut D,
_: server_sys::ClientId,
_: server_sys::ObjectId,
) {
}
}

expand_test!(protocol_error_in_request_without_object_init, {
Expand Down
9 changes: 6 additions & 3 deletions wayland-server/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@

## Unreleased

- Add `flush_clients` method to server `DisplayHandle`.
- Implement `AsFd` for `Display` so it can easily be used in a `calloop` source.

#### Breaking changes

- Bump bitflags to 2.0
- Updated wayland-backend to 0.2
- Use `BorrowedFd<'_>` arguments instead of `RawFd`
- `Resource::destroyed` now passes the resource type instead of the `ObjectId`.

#### Additions

- Add `flush_clients` method to server `DisplayHandle`.
- Implement `AsFd` for `Display` so it can easily be used in a `calloop` source.

#### Bugfixes

Expand Down
26 changes: 20 additions & 6 deletions wayland-server/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
///
/// To provide generic handlers for downstream usage, it is possible to make an implementation of the trait
/// that is generic over the last type argument, as illustrated below. Users will then be able to
/// automatically delegate their implementation to yours using the [`delegate_dispatch!`] macro.

Check warning on line 34 in wayland-server/src/dispatch.rs

View workflow job for this annotation

GitHub Actions / Documentation on Github Pages

unresolved link to `delegate_dispatch`
///
/// As a result, when your implementation is instanciated, the last type parameter `State` will be the state
/// struct of the app using your generic implementation. You can put additional trait constraints on it to
Expand Down Expand Up @@ -111,7 +111,13 @@
/// convenience.
///
/// By default this method does nothing.
fn destroyed(_state: &mut State, _client: ClientId, _resource: ObjectId, _data: &UserData) {}
fn destroyed(
_state: &mut State,
_client: wayland_backend::server::ClientId,
_resource: &I,
_data: &UserData,
) {
}
}

/// The [`ObjectData`] implementation that is internally used by this crate
Expand Down Expand Up @@ -270,12 +276,20 @@
}

fn destroyed(
&self,
self: Arc<Self>,
handle: &wayland_backend::server::Handle,
data: &mut D,
client_id: wayland_backend::server::ClientId,
object_id: wayland_backend::server::ObjectId,
client_id: ClientId,
object_id: ObjectId,
) {
<D as Dispatch<I, U>>::destroyed(data, client_id, object_id, &self.udata)
let dhandle = DisplayHandle::from(handle.clone());
let mut resource = I::from_id(&dhandle, object_id).unwrap();

// Proxy::from_id will return an inert protocol object wrapper inside of ObjectData::destroyed,
// therefore manually initialize the data associated with protocol object wrapper.
resource.__set_object_data(self.clone());

<D as Dispatch<I, U>>::destroyed(data, client_id, &resource, &self.udata)
}
}

Expand Down Expand Up @@ -349,7 +363,7 @@
<$dispatch_to as $crate::Dispatch<$interface, $udata, Self>>::request(state, client, resource, request, data, dhandle, data_init)
}

fn destroyed(state: &mut Self, client: $crate::backend::ClientId, resource: $crate::backend::ObjectId, data: &$udata) {
fn destroyed(state: &mut Self, client: $crate::backend::ClientId, resource: &$interface, data: &$udata) {

Check warning on line 366 in wayland-server/src/dispatch.rs

View check run for this annotation

Codecov / codecov/patch

wayland-server/src/dispatch.rs#L366

Added line #L366 was not covered by tests
<$dispatch_to as $crate::Dispatch<$interface, $udata, Self>>::destroyed(state, client, resource, data)
}
}
Expand Down
9 changes: 8 additions & 1 deletion wayland-server/src/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,14 @@ impl<D> ObjectData<D> for ProtocolErrorData {
None
}

fn destroyed(&self, _data: &mut D, _client_id: ClientId, _object_id: ObjectId) {}
fn destroyed(
self: Arc<Self>,
_handle: &Handle,
_data: &mut D,
_client_id: ClientId,
_object_id: ObjectId,
) {
}
}

/// A trait which provides an implementation for handling advertisement of a global to clients with some type
Expand Down
4 changes: 2 additions & 2 deletions wayland-tests/tests/destructors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ impl ways::Dispatch<ways::protocol::wl_output::WlOutput, ServerUData> for Server

fn destroyed(
_: &mut Self,
_client: wayland_backend::server::ClientId,
_resource: wayland_backend::server::ObjectId,
_: ways::backend::ClientId,
_resource: &ways::protocol::wl_output::WlOutput,
data: &ServerUData,
) {
data.0.store(true, Ordering::Release);
Expand Down
Loading