Skip to content

Commit

Permalink
implement the reentrant instance storage
Browse files Browse the repository at this point in the history
  • Loading branch information
lilizoey committed Nov 26, 2023
1 parent 76cb6b2 commit 7d93a64
Show file tree
Hide file tree
Showing 14 changed files with 737 additions and 58 deletions.
110 changes: 110 additions & 0 deletions .github/workflows/full-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ name: Full CI
# Runs before merging. Rebases on master to make sure CI passes for latest integration, not only for the PR at the time of creation.

on:
push:
merge_group:
# push:

Expand Down Expand Up @@ -163,25 +164,50 @@ jobs:
artifact-name: macos-nightly
godot-binary: godot.macos.editor.dev.x86_64
rust-extra-args: --features godot/custom-godot

- name: macos-bind-self
os: macos-12
artifact-name: macos-nightly
godot-binary: godot.macos.editor.dev.x86_64
rust-extra-args: --features godot/custom-godot,itest/experimental-bind-self

- name: macos-double
os: macos-12
artifact-name: macos-double-nightly
godot-binary: godot.macos.editor.dev.double.x86_64
rust-extra-args: --features godot/custom-godot,godot/double-precision

- name: macos-double-bind-self
os: macos-12
artifact-name: macos-double-nightly
godot-binary: godot.macos.editor.dev.double.x86_64
rust-extra-args: --features godot/custom-godot,godot/double-precision,itest/experimental-bind-self

- name: macos-4.1
os: macos-12
artifact-name: macos-stable
godot-binary: godot.macos.editor.dev.x86_64
#godot-prebuilt-patch: '4.1'
rust-extra-args: --features itest/experimental-bind-self

- name: macos-bind-self-4.1
os: macos-12
artifact-name: macos-stable
godot-binary: godot.macos.editor.dev.x86_64
#godot-prebuilt-patch: '4.1'

# TODO merge with other jobs
- name: macos-lazy-fptrs
os: macos-12
artifact-name: macos-nightly
godot-binary: godot.macos.editor.dev.x86_64
rust-extra-args: --features godot/lazy-function-tables

- name: macos-lazy-fptrs-bind-self
os: macos-12
artifact-name: macos-nightly
godot-binary: godot.macos.editor.dev.x86_64
rust-extra-args: --features godot/lazy-function-tables,itest/experimental-bind-self

# Windows

Expand All @@ -191,24 +217,49 @@ jobs:
godot-binary: godot.windows.editor.dev.x86_64.exe
rust-extra-args: --features godot/custom-godot

- name: windows-bind-self
os: windows-latest
artifact-name: windows-nightly
godot-binary: godot.windows.editor.dev.x86_64.exe
rust-extra-args: --features godot/custom-godot,itest/experimental-bind-self

- name: windows-double
os: windows-latest
artifact-name: windows-double-nightly
godot-binary: godot.windows.editor.dev.double.x86_64.exe
rust-extra-args: --features godot/custom-godot,godot/double-precision

- name: windows-double-bind-self
os: windows-latest
artifact-name: windows-double-nightly
godot-binary: godot.windows.editor.dev.double.x86_64.exe
rust-extra-args: --features godot/custom-godot,godot/double-precision,itest/experimental-bind-self

- name: windows-4.1
os: windows-latest
artifact-name: windows-stable
godot-binary: godot.windows.editor.dev.x86_64.exe
#godot-prebuilt-patch: '4.1'

- name: windows-bind-self-4.1
os: windows-latest
artifact-name: windows-stable
godot-binary: godot.windows.editor.dev.x86_64.exe
#godot-prebuilt-patch: '4.1'
rust-extra-args: --features itest/experimental-bind-self

# TODO merge with other jobs
- name: windows-lazy-fptrs
os: windows-latest
artifact-name: windows-nightly
godot-binary: godot.windows.editor.dev.x86_64.exe
rust-extra-args: --features godot/lazy-function-tables

- name: windows-lazy-fptrs-bind-self
os: windows-latest
artifact-name: windows-nightly
godot-binary: godot.windows.editor.dev.x86_64.exe
rust-extra-args: --features godot/lazy-function-tables,itest/experimental-bind-self

# Linux

Expand All @@ -220,12 +271,24 @@ jobs:
godot-binary: godot.linuxbsd.editor.dev.x86_64
rust-extra-args: --features godot/custom-godot

- name: linux-bind-self
os: ubuntu-20.04
artifact-name: linux-nightly
godot-binary: godot.linuxbsd.editor.dev.x86_64
rust-extra-args: --features godot/custom-godot,itest/experimental-bind-self

- name: linux-double
os: ubuntu-20.04
artifact-name: linux-double-nightly
godot-binary: godot.linuxbsd.editor.dev.double.x86_64
rust-extra-args: --features godot/custom-godot,godot/double-precision

- name: linux-double-bind-self
os: ubuntu-20.04
artifact-name: linux-double-nightly
godot-binary: godot.linuxbsd.editor.dev.double.x86_64
rust-extra-args: --features godot/custom-godot,godot/double-precision,itest/experimental-bind-self

- name: linux-features
os: ubuntu-20.04
artifact-name: linux-nightly
Expand All @@ -239,6 +302,12 @@ jobs:
godot-binary: godot.linuxbsd.editor.dev.x86_64
rust-extra-args: --features godot/lazy-function-tables

- name: linux-lazy-fptrs-bind-self
os: ubuntu-20.04
artifact-name: linux-nightly
godot-binary: godot.linuxbsd.editor.dev.x86_64
rust-extra-args: --features godot/lazy-function-tables,itest/experimental-bind-self

# Linux compat

- name: linux-4.1.1
Expand All @@ -247,18 +316,38 @@ jobs:
godot-binary: godot.linuxbsd.editor.dev.x86_64
#godot-prebuilt-patch: '4.1.1'

- name: linux-bind-self-4.1.1
os: ubuntu-20.04
artifact-name: linux-stable
godot-binary: godot.linuxbsd.editor.dev.x86_64
#godot-prebuilt-patch: '4.1.1'
rust-extra-args: --features itest/experimental-bind-self

- name: linux-4.1
os: ubuntu-20.04
artifact-name: linux-stable
godot-binary: godot.linuxbsd.editor.dev.x86_64
godot-prebuilt-patch: '4.1'

- name: linux-bind-self-4.1
os: ubuntu-20.04
artifact-name: linux-stable
godot-binary: godot.linuxbsd.editor.dev.x86_64
godot-prebuilt-patch: '4.1'
rust-extra-args: --features itest/experimental-bind-self

- name: linux-4.0.4
os: ubuntu-20.04
artifact-name: linux-4.0.4
godot-binary: godot.linuxbsd.editor.dev.x86_64
godot-prebuilt-patch: '4.0.4'

- name: linux-bind-self-4.0.4
os: ubuntu-20.04
artifact-name: linux-4.0.4
godot-binary: godot.linuxbsd.editor.dev.x86_64
godot-prebuilt-patch: '4.0.4'
rust-extra-args: --features itest/experimental-bind-self

# Memory checks: special Godot binaries compiled with AddressSanitizer/LeakSanitizer to detect UB/leaks.
# See also https://rustc-dev-guide.rust-lang.org/sanitizers.html.
Expand All @@ -277,6 +366,16 @@ jobs:
# Sanitizers can't build proc-macros and build scripts; with --target, cargo ignores RUSTFLAGS for those two.
rust-target: x86_64-unknown-linux-gnu

- name: linux-memcheck-bind-self
os: ubuntu-20.04
artifact-name: linux-memcheck-clang-nightly
godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san
rust-toolchain: nightly
rust-env-rustflags: -Zrandomize-layout -Zsanitizer=address
rust-extra-args: --features godot/custom-godot,itest/experimental-bind-self
# Sanitizers can't build proc-macros and build scripts; with --target, cargo ignores RUSTFLAGS for those two.
rust-target: x86_64-unknown-linux-gnu

- name: linux-memcheck-4.0.4
os: ubuntu-20.04
artifact-name: linux-memcheck-clang-4.0.4
Expand All @@ -286,6 +385,17 @@ jobs:
rust-env-rustflags: -Zrandomize-layout -Zsanitizer=address
# Sanitizers can't build proc-macros and build scripts; with --target, cargo ignores RUSTFLAGS for those two.
rust-target: x86_64-unknown-linux-gnu

- name: linux-memcheck-bind-self-4.0.4
os: ubuntu-20.04
artifact-name: linux-memcheck-clang-4.0.4
godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san
godot-prebuilt-patch: '4.0.4'
rust-toolchain: nightly
rust-env-rustflags: -Zrandomize-layout -Zsanitizer=address
rust-extra-args: --features itest/experimental-bind-self
# Sanitizers can't build proc-macros and build scripts; with --target, cargo ignores RUSTFLAGS for those two.
rust-target: x86_64-unknown-linux-gnu

steps:
- uses: actions/checkout@v3
Expand Down
2 changes: 2 additions & 0 deletions godot-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ custom-godot = ["godot-ffi/custom-godot", "godot-codegen/custom-godot"]
double-precision = ["godot-codegen/double-precision"]
experimental-godot-api = ["godot-codegen/experimental-godot-api"]
experimental-threads = []
experimental-bind-self = ["gd-cell"]
trace = ["godot-ffi/trace"]

[dependencies]
Expand All @@ -27,6 +28,7 @@ godot-ffi = { path = "../godot-ffi" }
# See https://docs.rs/glam/latest/glam/index.html#feature-gates
glam = { version = "0.23", features = ["debug-glam-assert"] }
serde = { version = "1", features = ["derive"], optional = true }
gd-cell = { git = "https://github.com/lilizoey/gd-cell", branch = "main", optional = true }

# Reverse dev dependencies so doctests can use `godot::` prefix
[dev-dependencies]
Expand Down
2 changes: 2 additions & 0 deletions godot-core/src/obj/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ impl<T: GodotClass> Base<T> {
///
/// Using this method to call methods on the base field of a Rust object is discouraged, instead use the
/// methods from [`WithBaseField`](super::WithBaseField) when possible.
#[doc(hidden)]
pub fn to_gd(&self) -> Gd<T> {
(*self.obj).clone()
}
Expand All @@ -73,6 +74,7 @@ impl<T: GodotClass> Base<T> {
///
/// Using this method to call methods on the base field of a Rust object is discouraged, instead use the
/// methods from [`WithBaseField`](super::WithBaseField) when possible.
#[doc(hidden)]
pub fn as_gd(&self) -> &Gd<T> {
&self.obj
}
Expand Down
54 changes: 43 additions & 11 deletions godot-core/src/obj/guards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@
use godot_ffi::out;

use std::fmt::Debug;
use std::marker::PhantomData;
use std::ops::{Deref, DerefMut};

use crate::storage::{MutGuard, RefGuard};
use crate::storage::{BaseMutGuard, MutGuard, RefGuard};

use super::{Gd, GodotClass};

Expand Down Expand Up @@ -80,34 +79,67 @@ impl<T: GodotClass> Drop for GdMut<'_, T> {
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------

/// Shared reference guard for a [`Base`](crate::obj::Base) pointer.
///
/// This can be used to call methods on the base object of a rust object that take `&self` as the receiver.
///
/// See [`WithBaseField::base()`](super::WithBaseField::base()) for usage.
pub struct BaseRef<'a, T: GodotClass> {
pub(crate) gd: Gd<T>,
pub(crate) _p: PhantomData<&'a ()>,
gd: Gd<T::Base>,
_instance: &'a T,
}

impl<'a, T: GodotClass> BaseRef<'a, T> {
pub(crate) fn new(gd: Gd<T::Base>, instance: &'a T) -> Self {
Self {
gd,
_instance: instance,
}
}
}

impl<T: GodotClass> Deref for BaseRef<'_, T> {
type Target = Gd<T>;
type Target = Gd<T::Base>;

fn deref(&self) -> &Gd<T> {
fn deref(&self) -> &Gd<T::Base> {
&self.gd
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------

/// Mutable/exclusive reference guard for a [`Base`](crate::obj::Base) pointer.
///
/// This can be used to call methods on the base object of a rust object that take `&self` or `&mut self` as
/// the receiver.
///
/// See [`WithBaseField::base_mut()`](super::WithBaseField::base_mut()) for usage.
pub struct BaseMut<'a, T: GodotClass> {
pub(crate) gd: Gd<T>,
pub(crate) _p: PhantomData<&'a mut ()>,
gd: Gd<T::Base>,
_base_mut_guard: BaseMutGuard<'a, T>,
}

impl<'a, T: GodotClass> BaseMut<'a, T> {
pub(crate) fn new(gd: Gd<T::Base>, base_mut_guard: BaseMutGuard<'a, T>) -> Self {
Self {
gd,
_base_mut_guard: base_mut_guard,
}
}
}

impl<T: GodotClass> Deref for BaseMut<'_, T> {
type Target = Gd<T>;
type Target = Gd<T::Base>;

fn deref(&self) -> &Gd<T> {
fn deref(&self) -> &Gd<T::Base> {
&self.gd
}
}

impl<T: GodotClass> DerefMut for BaseMut<'_, T> {
fn deref_mut(&mut self) -> &mut Gd<T> {
fn deref_mut(&mut self) -> &mut Gd<T::Base> {
&mut self.gd
}
}
27 changes: 26 additions & 1 deletion godot-core/src/obj/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,16 +285,41 @@ where
/// Storage object associated with the extension instance.
///
/// Returns `None` if self is null.
pub(crate) fn storage(&self) -> Option<&InstanceStorage<T>> {
pub(crate) fn storage<'a>(&'a self) -> Option<&'a InstanceStorage<T>> {
// SAFETY:
// - We have a `&self`, so the storage must already have been created.
// - The storage cannot be destroyed while we have a `&self` reference, so it will not be
// destroyed for the duration of `'a`.
unsafe { self.storage_unbounded() }
}

/// Storage object associated with the extension instance.
///
/// Returns `None` if self is null.
///
/// # Safety
///
/// This method provides a reference to the storage with an arbitrarily long lifetime `'b`. The reference
/// must actually be live for the duration of this lifetime.
///
/// The only time when a `&mut` reference can be taken to a `InstanceStorage` is when it is constructed
/// or destroyed. So it is sufficient to ensure that the storage is not created or destroyed during the
/// lifetime `'b`.
pub(crate) unsafe fn storage_unbounded<'a, 'b>(&'a self) -> Option<&'b InstanceStorage<T>> {
// SAFETY: instance pointer belongs to this instance. We only get a shared reference, no exclusive access, so even
// calling this from multiple Gd pointers is safe.
//
// The caller is responsible for ensuring that the storage is live for the duration of the lifetime
// `'b`.
//
// Potential issue is a concurrent free() in multi-threaded access; but that would need to be guarded against inside free().
unsafe {
let binding = self.resolve_instance_ptr();
sys::ptr_then(binding, |binding| crate::private::as_storage::<T>(binding))
}
}

// TODO: document unsafety in this function, and double check that it actually needs to be unsafe.
unsafe fn resolve_instance_ptr(&self) -> sys::GDExtensionClassInstancePtr {
if self.is_null() {
return ptr::null_mut();
Expand Down
Loading

0 comments on commit 7d93a64

Please sign in to comment.