-
Notifications
You must be signed in to change notification settings - Fork 450
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
unexpected bug on paralellization of patterns #1083
Comments
I need a reproduction please.
It was usually working fine with regex 1.8? Or was always working correctly? |
It was usually working fine with regex 1.8? Or was always working correctly? It is working battle tested through really too many batch heavy processes on the dependency v1.8 in a flawless manner (not a single panic). I will try to create something similar, I guess it will take me time to reach a reproducible piece of code. It is by the way being called trhough pyo3 with allow_threads, maybe something is hitting after the change with this. I will update here as soon as possible with an example breaking this parallelism. Kind regards |
Okay, if there wasn't a single panic then it "always worked," not "usually worked." |
And yes, a reproduction would be extremely helpful here. |
trying to reproduce it but only appears on one exact combo of patterns, if I subset them into distinct groups it does not appear. In fact it works perfectly in any other combination of groups of patterns of the same set (this gives me headache because I was supposing to have a rotten apple there). It does not look like an OOM problem, it is far from OOM and it is not a 143 exit. Trying to generate the example, I do not have any clue about how to reproduce it without giving exact data (what I can not do). Could you give me any clue about what is this panic about?
The assert breaking is this one https://docs.rs/regex-automata/0.3.8/src/regex_automata/hybrid/dfa.rs.html#2563 Any clue would be very nice to try to build something that breaks it in a generalist manner. By the way, the kind of load in work here is somehow reproduced here (with haystack and patterns to be provided) https://gist.github.com/Guillermogsjc/74874a806e5d1006ec7075ddffd5f25a#file-load_test-rs it is not exactly this , but close (it is launched from a pyo3 function with allow_threads). Thanks in advance. |
I don't know what's causing it sadly and I don't know how to contextualize that assert in a way that relates to the public API. If I could do that, I probably wouldn't need a reproduction. |
is here maybe a bad practice or usage of the regex crate inside the rayon parallel scheme in such a way it is prone to throw this panic ? https://gist.github.com/Guillermogsjc/74874a806e5d1006ec7075ddffd5f25a#file-load_test-rs |
Nope. The panic is definitively a bug. Regex find routines should never panic under any circumstances (except for perhaps exhausting heap memory, but I don't think that's relevant here). |
I tried looking into this to see if I could figure out how you're getting the panic, and I can't see it. My best guess is that there is some kind of bug in handling the lazy DFA's cache resets. I think the bottom line here is that I'm going to need a repro. If you could get me that, that would be wonderful and I'd be most appreciative. To be clear, a repro means that you can provide me a set of commands and inputs that I can run on my own machine and observe the same thing you're observing. So a repro means you'd provide me with a transcript of actions that I can take and see the problem for myself. |
Hi @BurntSushi , yes I know that we need a deterministic input+code combination to be able to dig into this bug. I have been trying several times with distinct shareable inputs against same flow, but so far did not break it. Will continue searching for this shareable inputs that reach same state, but as you well know, it is like searching a needle in a haystack, because do not have a remote clue about the characterization of the patterns that cause this. To be on track, this is about the patterns not about the haystack right? Or has to do with the combination? |
It is almost certainly a combination of both patterns and haystack. |
hi @BurntSushi , have been for a while trying to encrypt both patterns and haystack in a way that everything works similar and it breaks as breaks with original data, to provide reproduction of the bug. So far did not get a reliable encrypting that honors patterns match over haystack in a sufficient way (with only obfuscation i get it, but as i encrypt, the subword patterns stop matching and everything changes and does not break). The thing is that after upgrading rayon and regex crate, with the exact combination that I had issues... its is gone! rayon: 1.7.0 into 1.8.0 also removing those unneeded Arc over &str haystack vectors before feeding So if is ok for you, this issue would be better closed. |
Definitely sounds suspicious, but without a solid repro I agree with closing this. Feel free to re-open this if you rediscover the bug. |
Hi @BurntSushi, Thanks! |
What version of regex are you using?
Upgrading the dependency from
regex = "1.8"
towith
rayon = "1.7"
and compiled withrustc 1.71.1 (eb26296b5 2023-08-03)
Describe the bug at a high level.
Used parallelization on searches in battle tested way with the combo rayon -> 1.7 and regex -> 1.8, but after upgrading, the same code generates unexpected bug.
The paralellization is made by chunks of patterns, meaning that built regex are not shared across threads neither cloned, with something like:
where
BuiltRegex
is a struct that hasregex::RegexSet
compiled (built in each chunk of patterns <-> thread), and a vec ofregex::Regex
.Log:
What are the steps to reproduce the behavior?
It is very hard to generate reproducible steps.
What is the expected behavior?
It was usually working fine before change of minor from v1.8 to v1.9 on regex crate with default features.
On the Cargo.lock, i can see changes:
The text was updated successfully, but these errors were encountered: