Skip to content

Commit

Permalink
A strawfox for mozilla#417 to allow an interface to have a method whi…
Browse files Browse the repository at this point in the history
…ch takes an `Arc::clone(self)`

Intent is a new attribute is added to such methods (and the method must be on
a `[ThreadSafe]` interface). I've chosen `[SomethingSomethingArc]` as the
attribute name :)

I attempted to change things so handle_map.rs no longer does the
ref-deref `&*` magic but instead delegates that to the macros, which
only does it if the new attribute exists. This *nearly* works, but
has unintended consequences I don't yet understand (ie, the
non-threadsafe generated code is using `&*obj` which makes no sense
at this hour).
  • Loading branch information
mhammond committed Mar 16, 2021
1 parent a5b5a49 commit 4977a7a
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 6 deletions.
8 changes: 8 additions & 0 deletions examples/threadsafe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use std::sync::atomic::{AtomicBool, AtomicI32, Ordering};
use std::sync::Arc;

/// Simulation of a task doing something to keep a thread busy.
/// Up until now, everything has been synchronous and blocking, so
Expand Down Expand Up @@ -53,6 +54,7 @@ impl Counter {
// It relies on its own locking mechanisms, and uses the `Threadsafe`
// attribute to tell uniffi to use the `ArcHandleMap`, so avoid the default
// locking strategy.
#[derive(Debug)]
struct ThreadsafeCounter {
is_busy: AtomicBool,
count: AtomicI32,
Expand All @@ -62,6 +64,7 @@ struct ThreadsafeCounter {
// `Send` and `Sync`.
static_assertions::assert_impl_all!(ThreadsafeCounter: Send, Sync);


impl ThreadsafeCounter {
fn new() -> Self {
Self {
Expand All @@ -83,6 +86,11 @@ impl ThreadsafeCounter {
self.count.load(Ordering::SeqCst)
}
}

fn cloneable(self: Arc<Self>) {
// `self` is already an Arc::clone() so can be used directly.
println!("Got {:?}", self)
}
}

include!(concat!(env!("OUT_DIR"), "/threadsafe.uniffi.rs"));
4 changes: 4 additions & 0 deletions examples/threadsafe/src/threadsafe.udl
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,9 @@ interface Counter {
interface ThreadsafeCounter {
void busy_wait(i32 ms);
i32 increment_if_busy();
// A function that wants a signature of `fn cloneable(self: Arc<Self>)`
// so that it's capable of sending an Arc::clone somewhere else.
[SomethingSomethingArc] // probably want a better attribute? ;)
void cloneable();
};

10 changes: 5 additions & 5 deletions uniffi/src/ffi/handle_maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ impl<T: Sync + Send> ArcHandleMap<T> {
callback: F,
) -> R::Value
where
F: std::panic::UnwindSafe + FnOnce(&T) -> Result<R, E>,
F: std::panic::UnwindSafe + FnOnce(Arc<T>) -> Result<R, E>,
ExternError: From<E>,
R: IntoFfi,
{
Expand All @@ -175,7 +175,7 @@ impl<T: Sync + Send> ArcHandleMap<T> {
let obj = map.get(h)?;
Arc::clone(&obj)
};
Ok(callback(&*obj)?)
Ok(callback(obj)?)
})
}

Expand All @@ -187,7 +187,7 @@ impl<T: Sync + Send> ArcHandleMap<T> {
callback: F,
) -> R::Value
where
F: std::panic::UnwindSafe + FnOnce(&T) -> R,
F: std::panic::UnwindSafe + FnOnce(Arc<T>) -> R,
R: IntoFfi,
{
self.call_with_result(out_error, h, |r| -> Result<_, HandleError> {
Expand Down Expand Up @@ -313,7 +313,7 @@ impl<T: Sync + Send> ArcHandleMap<T> {
callback: F,
) -> R::Value
where
F: std::panic::UnwindSafe + FnOnce(&T) -> Result<R, E>,
F: std::panic::UnwindSafe + FnOnce(Arc<T>) -> Result<R, E>,
ExternError: From<E>,
R: IntoFfi,
{
Expand All @@ -327,7 +327,7 @@ impl<T: Sync + Send> ArcHandleMap<T> {
callback: F,
) -> R::Value
where
F: std::panic::UnwindSafe + FnOnce(&T) -> R,
F: std::panic::UnwindSafe + FnOnce(Arc<T>) -> R,
R: IntoFfi,
{
self.call_with_output(out_error, h, callback)
Expand Down
9 changes: 8 additions & 1 deletion uniffi_bindgen/src/interface/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ use anyhow::{bail, Result};
/// This is a convenience enum for parsing UDL attributes and erroring out if we encounter
/// any unsupported ones. These don't convert directly into parts of a `ComponentInterface`, but
/// may influence the properties of things like functions and arguments.
#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone, Hash, PartialEq)]
pub(super) enum Attribute {
ByRef,
Error,
Threadsafe,
Throws(String),
SomethingSomethingArc,
}

impl Attribute {
Expand All @@ -50,6 +51,7 @@ impl TryFrom<&weedle::attribute::ExtendedAttribute<'_>> for Attribute {
"ByRef" => Ok(Attribute::ByRef),
"Error" => Ok(Attribute::Error),
"Threadsafe" => Ok(Attribute::Threadsafe),
"SomethingSomethingArc" => Ok(Attribute::SomethingSomethingArc),
_ => anyhow::bail!("ExtendedAttributeNoArgs not supported: {:?}", (attr.0).0),
},
// Matches assignment-style attributes like ["Throws=Error"]
Expand Down Expand Up @@ -145,6 +147,10 @@ impl FunctionAttributes {
_ => None,
})
}

pub(super) fn get_something_something_arc(&self) -> bool {
self.0.iter().any(|a| *a == Attribute::SomethingSomethingArc)
}
}

impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for FunctionAttributes {
Expand All @@ -154,6 +160,7 @@ impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for FunctionAttribut
) -> Result<Self, Self::Error> {
let attrs = parse_attributes(weedle_attributes, |attr| match attr {
Attribute::Throws(_) => Ok(()),
Attribute::SomethingSomethingArc => Ok(()), // XXX - not valid in ctors?
_ => bail!(format!(
"{:?} not supported for functions, methods or constructors",
attr
Expand Down
4 changes: 4 additions & 0 deletions uniffi_bindgen/src/interface/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,10 @@ impl Method {
self.attributes.get_throws_err()
}

pub fn something_something_arc(&self) -> bool {
self.attributes.get_something_something_arc()
}

pub fn derive_ffi_func(&mut self, ci_prefix: &str, obj_prefix: &str) -> Result<()> {
self.ffi_func.name.push_str(ci_prefix);
self.ffi_func.name.push_str("_");
Expand Down
8 changes: 8 additions & 0 deletions uniffi_bindgen/src/templates/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,20 @@ use uniffi::UniffiMethodCall;
{% match meth.throws() -%}
{% when Some with (e) -%}
{{ this_handle_map }}.method_call_with_result(err, {{ meth.first_argument().name() }}, |obj| -> Result<{% call return_type_func(meth) %}, {{e}}> {
{%- if meth.something_something_arc() -%}
let _retval = {{ obj.name() }}::{%- call to_rs_call_with_prefix("obj", meth) -%}?;
{%- else -%}
let _retval = {{ obj.name() }}::{%- call to_rs_call_with_prefix("&*obj", meth) -%}?;
{%- endif -%}
Ok({% call ret(meth) %})
})
{% else -%}
{{ this_handle_map }}.method_call_with_output(err, {{ meth.first_argument().name() }}, |obj| {
{%- if meth.something_something_arc() -%}
let _retval = {{ obj.name() }}::{%- call to_rs_call_with_prefix("obj", meth) -%};
{%- else -%}
let _retval = {{ obj.name() }}::{%- call to_rs_call_with_prefix("&*obj", meth) -%};
{%- endif -%}
{% call ret(meth) %}
})
{% endmatch -%}
Expand Down

0 comments on commit 4977a7a

Please sign in to comment.