Repository navigation
Closed
Labels
Description
The following minimal test case fails on regex 1.1.6:
fn main() {
let rs = regex::RegexSet::new(vec![""]).unwrap();
let rrs = &rs;
std::panic::catch_unwind(move || rrs.matches(""));
}
with the following error output:
error[E0277]: the type `std::cell::UnsafeCell<std::option::Option<std::boxed::Box<std::cell::RefCell<regex::exec::ProgramCacheInner>>>>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
--> src/main.rs:4:5
|
4 | std::panic::catch_unwind(move || rrs.matches(""));
| ^^^^^^^^^^^^^^^^^^^^^^^^ `std::cell::UnsafeCell<std::option::Option<std::boxed::Box<std::cell::RefCell<regex::exec::ProgramCacheInner>>>>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
|
= help: within `regex::re_set::unicode::RegexSet`, the trait `std::panic::RefUnwindSafe` is not implemented for `std::cell::UnsafeCell<std::option::Option<std::boxed::Box<std::cell::RefCell<regex::exec::ProgramCacheInner>>>>`
= note: required because it appears within the type `thread_local::CachedThreadLocal<std::cell::RefCell<regex::exec::ProgramCacheInner>>`
= note: required because it appears within the type `regex::exec::Exec`
= note: required because it appears within the type `regex::re_set::unicode::RegexSet`
= note: required because of the requirements on the impl of `std::panic::UnwindSafe` for `®ex::re_set::unicode::RegexSet`
= note: required because it appears within the type `[closure@src/main.rs:4:30: 4:53 rrs:®ex::re_set::unicode::RegexSet]`
= note: required by `std::panic::catch_unwind`
error[E0277]: the type `std::cell::UnsafeCell<regex::exec::ProgramCacheInner>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
--> src/main.rs:4:5
|
4 | std::panic::catch_unwind(move || rrs.matches(""));
| ^^^^^^^^^^^^^^^^^^^^^^^^ `std::cell::UnsafeCell<regex::exec::ProgramCacheInner>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
|
= help: within `regex::re_set::unicode::RegexSet`, the trait `std::panic::RefUnwindSafe` is not implemented for `std::cell::UnsafeCell<regex::exec::ProgramCacheInner>`
= note: required because it appears within the type `std::cell::RefCell<regex::exec::ProgramCacheInner>`
= note: required because it appears within the type `std::marker::PhantomData<std::cell::RefCell<regex::exec::ProgramCacheInner>>`
= note: required because it appears within the type `thread_local::ThreadLocal<std::cell::RefCell<regex::exec::ProgramCacheInner>>`
= note: required because it appears within the type `thread_local::CachedThreadLocal<std::cell::RefCell<regex::exec::ProgramCacheInner>>`
= note: required because it appears within the type `regex::exec::Exec`
= note: required because it appears within the type `regex::re_set::unicode::RegexSet`
= note: required because of the requirements on the impl of `std::panic::UnwindSafe` for `®ex::re_set::unicode::RegexSet`
= note: required because it appears within the type `[closure@src/main.rs:4:30: 4:53 rrs:®ex::re_set::unicode::RegexSet]`
= note: required by `std::panic::catch_unwind`
error[E0277]: the type `std::cell::UnsafeCell<isize>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
--> src/main.rs:4:5
|
4 | std::panic::catch_unwind(move || rrs.matches(""));
| ^^^^^^^^^^^^^^^^^^^^^^^^ `std::cell::UnsafeCell<isize>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
|
= help: within `regex::re_set::unicode::RegexSet`, the trait `std::panic::RefUnwindSafe` is not implemented for `std::cell::UnsafeCell<isize>`
= note: required because it appears within the type `std::cell::Cell<isize>`
= note: required because it appears within the type `std::cell::RefCell<regex::exec::ProgramCacheInner>`
= note: required because it appears within the type `std::marker::PhantomData<std::cell::RefCell<regex::exec::ProgramCacheInner>>`
= note: required because it appears within the type `thread_local::ThreadLocal<std::cell::RefCell<regex::exec::ProgramCacheInner>>`
= note: required because it appears within the type `thread_local::CachedThreadLocal<std::cell::RefCell<regex::exec::ProgramCacheInner>>`
= note: required because it appears within the type `regex::exec::Exec`
= note: required because it appears within the type `regex::re_set::unicode::RegexSet`
= note: required because of the requirements on the impl of `std::panic::UnwindSafe` for `®ex::re_set::unicode::RegexSet`
= note: required because it appears within the type `[closure@src/main.rs:4:30: 4:53 rrs:®ex::re_set::unicode::RegexSet]`
= note: required by `std::panic::catch_unwind`
error: aborting due to 3 previous errors
For more information about this error, try `rustc --explain E0277`.
error: Could not compile `testapp`.
To learn more, run the command again with --verbose.
Activity
bdonlan commentedon Apr 18, 2019
Note that this is an issue for Regex types as well, not just RegexSet.
BurntSushi commentedon Apr 18, 2019
Did this use to work? (I don't think it did.) So are you reported this as a feature request?
It's not immediately obvious to me that this should work. But it's probably okay for regex internals to assert that it is unwind safe since interior mutability is never used for correctness, only performance.
bdonlan commentedon Apr 19, 2019
I'm not sure if this was supported in the past; it was a bit of a sharp edge that bit me, as I'd assume normally that immutable objects would be unwind safe. If the interior mutability is only used for caching, perhaps the safest approach would be to drop the cache if a panic occurs inside the regex engine?
regex: support perf-cache
regex: support perf-cache
regex: support perf-cache
rtkaratekid commentedon Aug 31, 2020
Just wanted to chime in that when using libfuzz or afl.rs this is also an issue because you need std::panic::RefUnwindSafe implemented for objects within the closure. If you're trying to fuzz something that involves regex matching, you just wont be able to (at least from my rust newbie perspective).
dpathakj commentedon Feb 27, 2021
The situation may have changed here because of changes upstream in thread_local. The statement in the
#[cfg(not(feature = "perf-cache"))]impl ofCached<T>states that "CachedThreadLocal impls Send, Sync and UnwindSafe, but NOT RefUnwindSafe." That doesn't appear to be the case on tip of master, because (Cached)ThreadLocal is now RefUnwindSafe. If you remove that PhantomData you'll get an implementation that passes the testcase above regardless of whetherperf-cacheis enabled. (Undoing f6f8276 breaks it again.) Should the PhantomData be removed or is there another reason specific to this crate that it should remain?(I noticed this because the build is broken due to a warning. CachedThreadLocal is deprecated and now just passes through to ThreadLocal. That relates to this issue because the fix (just use ThreadLocal) should probably also adjust the comment, but just replacing "CachedThreadLocal" with "ThreadLocal" in the comment doesn't seem to make it right.)
BurntSushi commentedon Mar 1, 2021
@dpathakj Yeah, I'm working on addressing this. I have a newborn at home though, so it's tough. Basically, my plan here is to actually remove the
thread_localdependency. I've had this thought for a while since it is the root cause of some use cases resulting in bad memory leaks. Specifically, what happens is when a regex is used across lots of different threads and where (I suppose) some of those threads become idle but not actually destroyed, each thread winds up having a copy mutable scratch space used during matching. There have been a few issues about it I think. This one comes to mind: BurntSushi/rure-go#3The main problem with removing the dependency is the reason why
thread_localexists in the first place: it's really fast. So I need to do a bit of a benchmark investigation to see what I can do to replace it without sacrificing too much performance. Ideally without using anyunsafecode, since the sort ofunsaferequired to makethread_localwork is really quite tricky from my perspective and I don't really trust myself to get it right without a lot of deep thought and effort that I don't really have time for at the moment.impl: drop thread_local dependency
impl: drop thread_local dependency
BurntSushi commentedon Mar 12, 2021
This should be fixed in
regex 1.4.4. All regex types now implRefUnwindSafe.Merge #393