This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fory.git
The following commit(s) were added to refs/heads/main by this push:
new 83522b197 fix(rust): correct send sync ownership (#4003)
83522b197 is described below
commit 83522b1972f1d959e20c8f3c3bc32fc4dce7aea0
Author: Shawn Yang <[email protected]>
AuthorDate: Mon Aug 31 01:15:10 2026 +0800
fix(rust): correct send sync ownership (#4003)
## Why?
## What does this PR do?
## Related issues
Closes #3996
## AI Contribution Checklist
- [ ] Substantial AI assistance was used in this PR: `yes` / `no`
- [ ] If `yes`, I included a completed [AI Contribution
Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs)
in this PR description and the required `AI Usage Disclosure`.
- [ ] If `yes`, my PR description includes the required `ai_review`
summary and screenshot evidence or equivalent persisted links of the
final clean AI review results from both fresh reviewers described in
`AI_POLICY.md`, the Fory-guided reviewer and the independent general
reviewer, on the current PR diff or current HEAD after the latest code
changes.
## Does this PR introduce any user-facing change?
- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?
## Benchmark
---
.agents/languages/rust.md | 1 +
rust/fory-core/src/context.rs | 18 ------------------
rust/fory-core/src/fory.rs | 8 ++++++++
rust/fory-core/src/resolver/ref_resolver.rs | 4 ----
rust/fory-core/src/resolver/type_resolver.rs | 6 ------
rust/tests/Cargo.toml | 1 +
rust/tests/tests/test_multi_thread.rs | 9 ++++++++-
7 files changed, 18 insertions(+), 29 deletions(-)
diff --git a/.agents/languages/rust.md b/.agents/languages/rust.md
index 2bb064356..2760a0df6 100644
--- a/.agents/languages/rust.md
+++ b/.agents/languages/rust.md
@@ -13,6 +13,7 @@ Load this file when changing `rust/` or Rust xlang behavior.
- Do not set `FORY_PANIC_ON_ERROR=1` when running the full Rust test suite,
because some tests assert on error contents.
- Avoid cosmetic filesystem or module churn when logical module names and call
sites are already stable.
- Operation contexts such as `ReadContext` and `WriteContext` should sit
beside the runtime facade and aggregate resolver, buffer, and config state;
they are not resolver-owned submodules.
+- `Fory` is the only cross-thread runtime owner. `TypeResolver`,
`ReadContext`, `WriteContext`, and `RefReader` must remain `!Send + !Sync`;
shared root operations deep-clone resolver state into thread-local contexts. Do
not restore unsafe auto-trait implementations on those internal owners or
replace their hot-path `Rc` values with `Arc` to make them shareable.
- Runtime carriers belong in `types/`, and schema or type-hash helpers belong
with metadata hashing rather than generic wire/type-id modules.
- Rust derive-generated runtime paths are owned by the selected runtime crate:
normal downstream crates depend on `fory`, and `fory-derive` must resolve that
facade with `proc-macro-crate` and emit through `fory::__private`; direct
lower-level crates may resolve `fory-core`. Do not add raw crate-path string
attributes such as `#[fory(crate = "...")]`.
- `fory-core` `macro_rules!` exports, including `register_trait_type!` and
helpers, must use `$crate` for runtime paths so facade re-exports stay hygienic.
diff --git a/rust/fory-core/src/context.rs b/rust/fory-core/src/context.rs
index 77aec8b05..8e37fa549 100644
--- a/rust/fory-core/src/context.rs
+++ b/rust/fory-core/src/context.rs
@@ -359,15 +359,6 @@ impl<'a> Drop for WriteContext<'a> {
}
}
-// Safety: WriteContext is only shared across threads via higher-level pooling
code that
-// ensures single-threaded access while the context is in use. Users must
never hold the same
-// instance on multiple threads simultaneously; that would violate the
invariants and result in
-// undefined behavior. Under that assumption, marking it Send/Sync is sound.
-#[allow(clippy::needless_lifetimes)]
-unsafe impl<'a> Send for WriteContext<'a> {}
-#[allow(clippy::needless_lifetimes)]
-unsafe impl<'a> Sync for WriteContext<'a> {}
-
/// Deserialization state container used on a single thread at a time.
/// Sharing the same instance across threads simultaneously causes undefined
behavior.
pub struct ReadContext<'a> {
@@ -391,15 +382,6 @@ pub struct ReadContext<'a> {
current_depth: u32,
}
-// Safety: ReadContext follows the same invariants as WriteContext—external
orchestrators ensure
-// single-threaded use. Concurrent access to the same instance across threads
is forbidden and
-// would result in undefined behavior. With exclusive use guaranteed, the
Send/Sync markers are safe
-// even though Rc is used internally.
-#[allow(clippy::needless_lifetimes)]
-unsafe impl<'a> Send for ReadContext<'a> {}
-#[allow(clippy::needless_lifetimes)]
-unsafe impl<'a> Sync for ReadContext<'a> {}
-
impl<'a> ReadContext<'a> {
pub fn new(type_resolver: TypeResolver, config: Config) -> ReadContext<'a>
{
ReadContext {
diff --git a/rust/fory-core/src/fory.rs b/rust/fory-core/src/fory.rs
index 86ae1377d..60534ff43 100644
--- a/rust/fory-core/src/fory.rs
+++ b/rust/fory-core/src/fory.rs
@@ -450,6 +450,14 @@ pub struct Fory {
config: Config,
}
+// Safety: Fory is the only cross-thread owner of its resolvers. Registration
requires exclusive
+// access, and root operations permanently freeze the registry before the
finalized resolver is
+// shared. Fory never exposes the Rc values in either resolver. Each thread
deep-clones the
+// finalized resolver into thread-local contexts, so Rc counts and mutable
context state remain
+// confined to one thread.
+unsafe impl Send for Fory {}
+unsafe impl Sync for Fory {}
+
impl Default for Fory {
fn default() -> Self {
Self::builder().build()
diff --git a/rust/fory-core/src/resolver/ref_resolver.rs
b/rust/fory-core/src/resolver/ref_resolver.rs
index 9fe532e50..9dcf3aec0 100644
--- a/rust/fory-core/src/resolver/ref_resolver.rs
+++ b/rust/fory-core/src/resolver/ref_resolver.rs
@@ -256,10 +256,6 @@ pub struct RefReader {
callbacks: Vec<UpdateCallback>,
}
-// danger but useful for multi-thread
-unsafe impl Send for RefReader {}
-unsafe impl Sync for RefReader {}
-
impl RefReader {
/// Creates a new RefReader instance.
pub fn new() -> Self {
diff --git a/rust/fory-core/src/resolver/type_resolver.rs
b/rust/fory-core/src/resolver/type_resolver.rs
index 611b35339..91fdd0a95 100644
--- a/rust/fory-core/src/resolver/type_resolver.rs
+++ b/rust/fory-core/src/resolver/type_resolver.rs
@@ -730,12 +730,6 @@ pub struct TypeResolver {
xlang: bool,
}
-// Safety: TypeResolver instances are only shared through higher-level
synchronization that
-// guarantees thread confinement for mutations, so marking them Send/Sync
preserves existing
-// invariants despite internal Rc usage.
-unsafe impl Send for TypeResolver {}
-unsafe impl Sync for TypeResolver {}
-
const NO_TYPE_ID: TypeId = TypeId::UNKNOWN;
impl Default for TypeResolver {
diff --git a/rust/tests/Cargo.toml b/rust/tests/Cargo.toml
index 80184ffa8..4e83c5663 100644
--- a/rust/tests/Cargo.toml
+++ b/rust/tests/Cargo.toml
@@ -28,6 +28,7 @@ fory-derive = { path = "../fory-derive" }
fory-external-model = { path = "../api-tests/external-model" }
num-bigint = "0.4"
+static_assertions = "1.1.0"
[features]
default = []
diff --git a/rust/tests/tests/test_multi_thread.rs
b/rust/tests/tests/test_multi_thread.rs
index fdc69a4bc..35004a81d 100644
--- a/rust/tests/tests/test_multi_thread.rs
+++ b/rust/tests/tests/test_multi_thread.rs
@@ -15,12 +15,19 @@
// specific language governing permissions and limitations
// under the License.
-use fory_core::Fory;
+use fory_core::{resolver::RefReader, Fory, ReadContext, TypeResolver,
WriteContext};
use fory_derive::ForyStruct;
+use static_assertions::{assert_impl_all, assert_not_impl_any};
use std::collections::HashSet;
use std::sync::{Arc, Barrier};
use std::thread;
+assert_impl_all!(Fory: Send, Sync);
+assert_not_impl_any!(TypeResolver: Send, Sync);
+assert_not_impl_any!(ReadContext<'static>: Send, Sync);
+assert_not_impl_any!(WriteContext<'static>: Send, Sync);
+assert_not_impl_any!(RefReader: Send, Sync);
+
#[test]
fn test_simple_multi_thread() {
let fory =
Arc::new(Fory::builder().xlang(false).compatible(false).build());
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
