Skip to content

fix(rust): deep clone TypeMeta to prevent UB in concurrent scenarios#3511

Merged
chaokunyang merged 1 commit intoapache:mainfrom
BaldDemian:main
Mar 25, 2026
Merged

fix(rust): deep clone TypeMeta to prevent UB in concurrent scenarios#3511
chaokunyang merged 1 commit intoapache:mainfrom
BaldDemian:main

Conversation

@BaldDemian
Copy link
Copy Markdown
Contributor

@BaldDemian BaldDemian commented Mar 25, 2026

Why?

As mentioned in #3490 (comment), one cargo bench case stably fails in the current code. After some digging, I think it results from this line:

type_meta_by_index: self.type_meta_by_index.clone(),

The clone() method for TypeResolver should return a deep clone:
/// # Returns
///
/// A deep clone of the TypeResolver with all internal Rc instances recreated.
/// This ensures thread safety when cloning from multiple threads simultaneously.

But the type_meta_by_index field has type Vec<Option<Rc<TypeMeta>>>, calling clone() on this type directly would lead to a shadow clone indeed.

The internal_type_info_by_id field in TypeResolver also has a similar type Vec<Option<Rc<TypeInfo>>> but it doesn't call clone() directly.

What does this PR do?

  • deep clone type_meta_by_index. Now the cargo bench case can run successfully and stably.

  • introduce bazel-setup GitHub action in C++ sanitizer job

  • the existing test cases in test_multi_thread.rs are a little too weak to expose tricky concurrent issues. I wrote a new test case copying the main logic of the failed cargo bench case. Without the fix, this new test case would fail with a high chance.

Related issues

No.

AI Contribution Checklist

No.

Does this PR introduce any user-facing change?

No.

Benchmark

No.

@BaldDemian BaldDemian reopened this Mar 25, 2026
Copy link
Copy Markdown
Collaborator

@chaokunyang chaokunyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@chaokunyang chaokunyang merged commit eae25b6 into apache:main Mar 25, 2026
75 of 132 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants