Skip to content

Commit

Permalink
Resolve concerns for the original PR
Browse files Browse the repository at this point in the history
- `set_executor` and `register_runtime` are changed to allow multiple
  calls, at the expense of slightly worse error messages when they aren't
  called as necessary.
- Renamed the top-level re-export to `tasks` instead of `asn`.
- Added documentation for previously undocumented public items.
  • Loading branch information
chitoyuu committed Oct 26, 2021
1 parent 7aa9bc5 commit 6a542b6
Show file tree
Hide file tree
Showing 11 changed files with 34 additions and 89 deletions.
10 changes: 5 additions & 5 deletions gdnative-async/Cargo.toml
Expand Up @@ -16,11 +16,11 @@ edition = "2018"
gdnative-derive = { path = "../gdnative-derive", version = "=0.9.3" }
gdnative-core = { path = "../gdnative-core", version = "=0.9.3" }
gdnative-bindings = { path = "../gdnative-bindings", version = "=0.9.3" }
futures-task = "0.3.13"
futures-task = "0.3.17"
atomic-waker = "1.0.0"
once_cell = "1.7.2"
thiserror = "1.0"
parking_lot = "0.11.0"
crossbeam-channel = "0.5.0"
once_cell = "1.8.0"
parking_lot = "0.11.2"
crossbeam-channel = "0.5.1"
crossbeam-utils = "0.8.5"

[build-dependencies]
38 changes: 7 additions & 31 deletions gdnative-async/src/executor.rs
@@ -1,45 +1,21 @@
use std::cell::Cell;

use futures_task::LocalSpawn;
use once_cell::unsync::OnceCell as UnsyncCell;
use thiserror::Error;

thread_local!(
static LOCAL_SPAWN: UnsyncCell<&'static dyn LocalSpawn> = UnsyncCell::new();
static LOCAL_SPAWN: Cell<Option<&'static dyn LocalSpawn>> = Cell::new(None);
);

/// Error returned by `set_*_executor` if an executor of the kind has already been set.
#[derive(Error, Debug)]
#[error("an executor is already set")]
pub struct SetExecutorError {
_private: (),
}

impl SetExecutorError {
fn new() -> Self {
SetExecutorError { _private: () }
}
}

pub(crate) fn local_spawn() -> Option<&'static dyn LocalSpawn> {
LOCAL_SPAWN.with(|cell| cell.get().copied())
LOCAL_SPAWN.with(|cell| cell.get())
}

/// Sets the global executor for the current thread to a `Box<dyn LocalSpawn>`. This value is leaked.
pub fn set_boxed_executor(sp: Box<dyn LocalSpawn>) -> Result<(), SetExecutorError> {
pub fn set_boxed_executor(sp: Box<dyn LocalSpawn>) {
set_executor(Box::leak(sp))
}

/// Sets the global executor for the current thread to a `&'static dyn LocalSpawn`.
pub fn set_executor(sp: &'static dyn LocalSpawn) -> Result<(), SetExecutorError> {
LOCAL_SPAWN.with(|cell| cell.set(sp).map_err(|_| SetExecutorError::new()))
}

/// Sets the global executor for the current thread with a function that will only be called
/// if an executor isn't set yet.
pub fn ensure_executor_with<F>(f: F)
where
F: FnOnce() -> &'static dyn LocalSpawn,
{
LOCAL_SPAWN.with(|cell| {
cell.get_or_init(f);
});
pub fn set_executor(sp: &'static dyn LocalSpawn) {
LOCAL_SPAWN.with(|cell| cell.set(Some(sp)))
}
3 changes: 2 additions & 1 deletion gdnative-async/src/future.rs
Expand Up @@ -20,7 +20,8 @@ pub(crate) fn make<T>() -> (Yield<T>, Resume<T>) {
(future, resume)
}

/// Signal
/// Future that can be `await`ed for a signal or a `resume` call from Godot. See
/// [`Context`](crate::Context) for methods that return this future.
pub struct Yield<T> {
waker: Arc<AtomicWaker>,
arg_recv: Receiver<T>,
Expand Down
5 changes: 3 additions & 2 deletions gdnative-async/src/lib.rs
Expand Up @@ -14,6 +14,7 @@ mod future;
mod method;
mod rt;

pub use executor::{ensure_executor_with, set_boxed_executor, set_executor, SetExecutorError};
pub use executor::{set_boxed_executor, set_executor};
pub use future::Yield;
pub use method::{Async, AsyncMethod, Spawner};
pub use rt::{register_runtime, terminate_runtime, Context, InitError};
pub use rt::{register_runtime, terminate_runtime, Context};
5 changes: 3 additions & 2 deletions gdnative-async/src/method.rs
Expand Up @@ -24,8 +24,8 @@ pub trait AsyncMethod<C: NativeClass>: Send + Sync + 'static {
/// Spawns the future for result of this method with `spawner`. This is done so
/// that implementors of this trait do not have to name their future types.
///
/// If the `spawner` object is not used, the method call will fail, output an error,
/// and return a `Nil` variant.
/// If the `spawner` object is not used, the Godot side of the call will fail, output an
/// error, and return a `Nil` variant.
fn spawn_with(&self, spawner: Spawner<'_, C>);

/// Returns an optional site where this method is defined. Used for logging errors in FFI wrappers.
Expand All @@ -37,6 +37,7 @@ pub trait AsyncMethod<C: NativeClass>: Send + Sync + 'static {
}
}

/// A helper structure for working around naming future types. See [`Spawner::spawn`].
pub struct Spawner<'a, C: NativeClass> {
sp: &'static dyn LocalSpawn,
ctx: Context,
Expand Down
37 changes: 7 additions & 30 deletions gdnative-async/src/rt.rs
Expand Up @@ -2,8 +2,6 @@ use std::marker::PhantomData;

use gdnative_bindings::Object;
use gdnative_core::object::SubClass;
use once_cell::sync::OnceCell;
use thiserror::Error;

use gdnative_core::core_types::{GodotError, Variant};
use gdnative_core::nativescript::export::InitHandle;
Expand All @@ -18,20 +16,6 @@ mod func_state;

use func_state::FuncState;

static REGISTRATION: OnceCell<()> = OnceCell::new();

#[derive(Debug, Error)]
#[error("async runtime must only be initialized once")]
pub struct InitError {
_private: (),
}

impl InitError {
fn new() -> Self {
InitError { _private: () }
}
}

/// Context for creating `yield`-like futures in async methods.
pub struct Context {
func_state: Instance<FuncState, Shared>,
Expand Down Expand Up @@ -108,22 +92,15 @@ impl Context {
}
}

pub fn register_runtime(handle: &InitHandle) -> Result<(), InitError> {
let mut called = false;

REGISTRATION.get_or_init(|| {
handle.add_class::<bridge::SignalBridge>();
handle.add_class::<func_state::FuncState>();
called = true;
});

if called {
Ok(())
} else {
Err(InitError::new())
}
/// Adds required supporting NativeScript classes to `handle`. This must be called once and
/// only once per initialization.
pub fn register_runtime(handle: &InitHandle) {
handle.add_class::<bridge::SignalBridge>();
handle.add_class::<func_state::FuncState>();
}

/// Releases all observers still in use. This should be called in the
/// `godot_gdnative_terminate` callback.
pub fn terminate_runtime() {
bridge::terminate();
}
10 changes: 2 additions & 8 deletions gdnative-async/src/rt/bridge.rs
Expand Up @@ -43,8 +43,7 @@ struct Entry {
resume: Resume<Vec<Variant>>,

// Just need to keep this alive.
#[allow(dead_code)]
obj: Instance<SignalBridge, Shared>,
_obj: Instance<SignalBridge, Shared>,
}

pub(super) struct SignalBridge {
Expand All @@ -69,11 +68,6 @@ impl SignalBridge {
signal: &str,
resume: Resume<Vec<Variant>>,
) -> Result<(), GodotError> {
assert!(
super::REGISTRATION.get().is_some(),
"async API must be registered before any async methods can be called"
);

let mut pool = BRIDGES.get_or_init(Mutex::default).lock();
let (id, bridge) = pool.free.pop().unwrap_or_else(|| {
let id = pool.next_id();
Expand All @@ -90,8 +84,8 @@ impl SignalBridge {
)?;

let entry = Entry {
obj: bridge,
resume,
_obj: bridge,
};

assert!(pool.busy.insert(id, entry).is_none());
Expand Down
5 changes: 0 additions & 5 deletions gdnative-async/src/rt/func_state.rs
Expand Up @@ -53,11 +53,6 @@ impl NativeClass for FuncState {

impl FuncState {
pub fn new() -> Instance<Self, Unique> {
assert!(
super::REGISTRATION.get().is_some(),
"async API must be registered before any async methods can be called"
);

Instance::emplace(FuncState {
kind: Kind::Pending,
})
Expand Down
2 changes: 1 addition & 1 deletion gdnative/src/lib.rs
Expand Up @@ -88,4 +88,4 @@ pub use gdnative_bindings as api;
#[doc(inline)]
#[cfg(feature = "async")]
/// Support for async code
pub use gdnative_async as asn;
pub use gdnative_async as tasks;
2 changes: 1 addition & 1 deletion test/src/lib.rs
Expand Up @@ -272,7 +272,7 @@ fn init(handle: InitHandle) {
}

fn terminate(_term_info: &gdnative::TerminateInfo) {
gdnative::asn::terminate_runtime();
gdnative::tasks::terminate_runtime();
}

gdnative::macros::godot_gdnative_init!();
Expand Down
6 changes: 3 additions & 3 deletions test/src/test_async.rs
@@ -1,7 +1,7 @@
use std::cell::RefCell;

use gdnative::asn::{Async, AsyncMethod, Spawner};
use gdnative::prelude::*;
use gdnative::tasks::{Async, AsyncMethod, Spawner};

pub(crate) fn run_tests() -> bool {
// Relevant tests in GDScript
Expand All @@ -15,8 +15,8 @@ thread_local! {
}

pub(crate) fn register(handle: InitHandle) {
gdnative::asn::register_runtime(&handle).unwrap();
gdnative::asn::set_executor(EXECUTOR.with(|e| *e)).unwrap();
gdnative::tasks::register_runtime(&handle);
gdnative::tasks::set_executor(EXECUTOR.with(|e| *e));

handle.add_class::<AsyncMethods>();
handle.add_class::<AsyncExecutorDriver>();
Expand Down

0 comments on commit 6a542b6

Please sign in to comment.