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

Internal panic in the engine #916

Closed
irevoire opened this issue Sep 25, 2024 · 8 comments · Fixed by #917
Closed

Internal panic in the engine #916

irevoire opened this issue Sep 25, 2024 · 8 comments · Fixed by #917
Labels

Comments

@irevoire
Copy link

irevoire commented Sep 25, 2024

Hello rhai team,

We've introduced the possibility to edit document by (rhai) function in Meilisearch a few months ago and a user reported an internal panic in rhai.
I didn’t have the time to try to reproduce the issue on my side yet, but I thought it could be nice for you to get the error with the line number in case it’s trivial to fix:

2024-09-25T07:11:29.563769Z ERROR meilisearch: info=panicked at /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/rhai-1.19.0/src/engine.rs:347:58: called `Option::unwrap()` on a `None` value

And here’s the original issue with the function if that helps: meilisearch/meilisearch#4956

Thanks for the awesome lib 🎉

@schungx
Copy link
Collaborator

schungx commented Sep 25, 2024

This seems to happen during a strings cache contention. I can see that, under sync, there is possibility that the strings cache is still locked by another process while somebody wants to write to it.

I can remove the panic by simply cloning the string value if the cache is busy.

@schungx
Copy link
Collaborator

schungx commented Sep 25, 2024

Can you check the latest drop to see if it happens again? I just pushed a fix.

meili-bors bot added a commit to meilisearch/meilisearch that referenced this issue Sep 26, 2024
4960: Update rhai r=dureuill a=irevoire

# Pull Request

## Related issue
Fixes #4956

A fix has been implemented in rhaiscript/rhai#916

## What does this PR do?
- Use the latest version of rhai containing the fix

Co-authored-by: Tamo <[email protected]>
@schungx schungx added the bug label Sep 27, 2024
@schungx schungx linked a pull request Sep 27, 2024 that will close this issue
@irevoire
Copy link
Author

Hey, after a lot of tests from ourselves and our users we didn’t reproduce this bug or anything similar, thanks for the patch!

Do you think you could make a new patch release this week so we’re not relying on the git repo + a rev?

@longzou
Copy link

longzou commented Oct 16, 2024

I encountered the same proble.
It points to this function:

pub fn get_interned_string(
    &self,
    string: impl AsRef<str> + Into<ImmutableString>,
) -> ImmutableString {
    match self.interned_strings {
        Some(ref interner) => locked_write(interner).unwrap().get(string),
        None => string.into(),
    }
}

the locked_write(interner) returns None.

@schungx
Copy link
Collaborator

schungx commented Oct 16, 2024

Please kindly check out the latest drop to see if it solves your problem.

I'm planning to make a new release soonish if everything is stable.

@schungx
Copy link
Collaborator

schungx commented Nov 26, 2024

@irevoire hope this is now resolved?

@irevoire
Copy link
Author

Hey, we've not heard of this issue again.
I cannot say for sure it's 100% solved since it's not our most used feature. But it's definitely probably solved ahah

@schungx
Copy link
Collaborator

schungx commented Dec 12, 2024

Great! So closing this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants