Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Rc to Arc feature in async and multithread context #620

Closed
azoyan opened this issue Dec 23, 2024 · 7 comments · Fixed by #622
Closed

Add Rc to Arc feature in async and multithread context #620

azoyan opened this issue Dec 23, 2024 · 7 comments · Fixed by #622
Assignees
Labels
area: library Improvements to the library API quality type: feature New feature or request
Milestone

Comments

@azoyan
Copy link
Contributor

azoyan commented Dec 23, 2024

Is it possible to add feature to replace Rc in inner structures to Arc to use in async context?

pub struct RateLimitBodyJsonPath {
    pub jsonpath: Vec<RsonpathEngine>
}
error[E0277]: `Rc<StringPattern>` cannot be shared between threads safely
   --> core/src/rule/mod.rs:339:21
    |
339 | /                     tokio::spawn(async move {
340 | |                         let mut guard = RULES.write().await;
341 | |                         guard.remove(tenant_clone.as_str());
342 | |                     });
    | |______________________^ `Rc<StringPattern>` cannot be shared between threads safely
    |
    = help: within `[(Rc<StringPattern>, rsonpath::automaton::State); 2]`, the trait `Sync` is not implemented for `Rc<StringPattern>`, which is required by `{async block@core/src/rule/mod.rs:339:34: 339:44}: Send`
    = note: required because it appears within the type `(Rc<StringPattern>, rsonpath::automaton::State)`
    = note: required because it appears within the type `[(Rc<StringPattern>, rsonpath::automaton::State); 2]`
    = note: required for `smallvec::SmallVecData<[(Rc<StringPattern>, rsonpath::automaton::State); 2]>` to implement `Sync`
@azoyan azoyan added the type: feature New feature or request label Dec 23, 2024
@github-actions github-actions bot added the acceptance: triage Waiting for owner's input label Dec 23, 2024
Copy link

Tagging @V0ldek for notifications

@V0ldek V0ldek added this to the lib v1.0.0 milestone Dec 23, 2024
@github-actions github-actions bot added acceptance: go ahead Reviewed, implementation can start and removed acceptance: triage Waiting for owner's input labels Dec 23, 2024
@V0ldek
Copy link
Member

V0ldek commented Dec 23, 2024

It's possible, but after the holiday break 😄

For now as a workaround you can make it Send and Sync with a wrapper and unsafe

struct RsonpathEngineWrap(RsonpathEngine);

unsafe impl Send for RsonpathEngineWrap {}
unsafe impl Sync for RsonpathEngineWrap {}

This is safe because

  1. RsonpathEngine is not Clone
  2. We never clone the Rcs inside the Automaton after construction and always pass the strings by reference.

In other words the Rcs are never cloned after the engine is built.

@V0ldek V0ldek self-assigned this Dec 23, 2024
@V0ldek V0ldek added the area: library Improvements to the library API quality label Dec 23, 2024
@github-project-automation github-project-automation bot moved this from Todo to Merged in Active rq development Dec 24, 2024
@github-actions github-actions bot removed the acceptance: go ahead Reviewed, implementation can start label Dec 24, 2024
@azoyan
Copy link
Contributor Author

azoyan commented Dec 24, 2024

Make release please) 9.3

@azoyan
Copy link
Contributor Author

azoyan commented Dec 24, 2024

Make release please) 9.3

@V0ldek

@V0ldek
Copy link
Member

V0ldek commented Dec 24, 2024

Pinging me in the morning of Christmas Eve is unlikely to actually speed up a release 😛

If you're blocked, point your rsonpath-lib to the latest commit:

rsonpath-lib = { git = "https://github.com/rsonquery/rsonpath.git", rev = "37e907684173637191e3b42ea525d9a5ef0ab208" }

@azoyan
Copy link
Contributor Author

azoyan commented Dec 24, 2024

Pinging me in the morning of Christmas Eve is unlikely to actually speed up a release 😛

If you're blocked, point your rsonpath-lib to the latest commit:

rsonpath-lib = { git = "https://github.com/rsonquery/rsonpath.git", rev = "37e907684173637191e3b42ea525d9a5ef0ab208" }

Yes, I already use something like this, but for work I need to use the release version.

Ok. I will be waiting.

🎅🏻 🎄 Merry Christmas! 🎁 ☃️

@azoyan
Copy link
Contributor Author

azoyan commented Dec 24, 2024

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: library Improvements to the library API quality type: feature New feature or request
Projects
Development

Successfully merging a pull request may close this issue.

2 participants