-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
crate::Options is not Sync
#104
Comments
Current workaround I'm using. I'll write this up in a pull request later. /// A markdown::Options wrapper that can be safely passed between threads.
/// Note that this struct cannot wrap Options with MDX parser functions
/// specified. In other words, `options.parse.mdx_ems_parse` and
/// `options.parse.mdx_expression_parse` must both be `None`.
///
/// # Examples
///
/// ```rust
/// use docgen::state::ThreadsafeOptions;
///
/// // Equivalent to `let options = ThreadsafeOptions::new(markdown::Options::default());`
/// let options = ThreadsafeOptions::default();
/// let app_state = Arc::new(options);
/// // `app_state` may now be used in a multi-threaded web server.
/// ```
#[derive(Debug, Default)]
pub struct ThreadsafeOptions(Options);
unsafe impl Send for ThreadsafeOptions {}
unsafe impl Sync for ThreadsafeOptions {}
impl ThreadsafeOptions {
/// Create a new `ThreadsafeOptions` wrapper around the provided `Options`
/// object.
///
/// **PANICS**: This function will panic if you attempt to instantiate a
/// new object with an MDX parser specified. In other words,
/// `options.parse.mdx_ems_parse` and `options.parse.mdx_expression_parse`
/// must both be `None`.
pub fn new(options: Options) -> Self {
if options.parse.mdx_esm_parse.is_some() || options.parse.mdx_expression_parse.is_some() {
panic!("Cannot instantiate markdown::Options with MDX parsers specified")
}
Self(options)
}
}
impl From<Options> for ThreadsafeOptions {
fn from(value: Options) -> Self {
Self::new(value)
}
}
impl Borrow<Options> for ThreadsafeOptions {
fn borrow(&self) -> &Options {
&self.0
}
} |
Welcome @stephenlf! 👋
This is an XY Problem.
Memory is allocated all over the place to do parsing and create a syntax tree. This library aims to be 100% pure safe rust, Reading between the lines a bit you seem to be trying to reuse options across multiple threads in Rust? Is that accurate? |
I am incorporating a Markdown parser into an Axum web server I am writing. Rather than reallocate an I understand not wanting any I realize now it was presumptuous of me to say that I'd "set up a pull request later." I forgot this wasn't one of my projects! Let me know if you'd like me to explore this issue more. |
Other than safe, this project is also no_std + alloc. It’s incredibly basic, Box and Vec but that’s it. Send/Sync/Arc etc is all much higher level. |
That's the approach I took in my codebase. However, I suspect that annotating the MDX functions something like // markdown-rs/src/util/mdx.rs
// ...
pub type EsmParse = dyn Fn(&str) -> Signal + Sync + Send;
// ...
pub type ExpressionParse = dyn Fn(&str, &ExpressionKind) -> Signal + Sync + Send;
// ... This would of course mean that users couldn't dynamically create their MDX parsers, though I can't see this being a practical issue for most use cases. This code compiles and passes all checks, though I'm not sure what other downstream effects something like this might have. One can imagine a |
I have some doubts (but unfounded). If it works with https://github.com/wooorm/mdxjs-rs I’d be open to it. Anyway, closing this for now. The safe part of this project is very important. It also seems like there is a tiny fraction of time spent on this, we can probably spend our time better on parsing memory/performance. If all of that is addressed and it works with mdxjs-rs, I’d be open to a PR and rediscussing it! Best! :) |
The
Options
struct is notSync
. This is an issue in web servers because it means we must reallocate an Options struct every time we parse something.I will work on this problem later.
Full compiler diagnostic:
The text was updated successfully, but these errors were encountered: