Skip to content

Commit

Permalink
syntax: tweak the "no stack overflow" test
Browse files Browse the repository at this point in the history
This test works by spinning up a thread with an atypically small stack
size, parsing a regex into an Ast and then dropping it. We use a small
stack size such that *if the Ast didn't have a custom Drop impl*, then
its default recursive Drop impl would overflow the stack. (If we don't
use a smaller stack size, then the default on some platforms is usually
quite large and might require a much larger Ast to provoke a failure.)

It turns out that the stack size we were using was quite tiny, and too
tiny for some platforms such as FreeBSD. We therefore increase it a
little bit, but not too much.

We do the same for the corresponding test for the custom Drop impl for
Hir.

Fixes #967
  • Loading branch information
BurntSushi committed Mar 21, 2023
1 parent a9b2e02 commit eb7fb73
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
13 changes: 12 additions & 1 deletion regex-syntax/src/ast/mod.rs
Expand Up @@ -1492,8 +1492,19 @@ mod tests {

// We run our test on a thread with a small stack size so we can
// force the issue more easily.
//
// NOTE(2023-03-21): It turns out that some platforms (like FreeBSD)
// will just barf with very small stack sizes. So we bump this up a bit
// to give more room to breath. When I did this, I confirmed that if
// I remove the custom `Drop` impl for `Ast`, then this test does
// indeed still fail with a stack overflow. (At the time of writing, I
// had to bump it all the way up to 32K before the test would pass even
// without the custom `Drop` impl. So 16K seems like a safe number
// here.)
//
// See: https://github.com/rust-lang/regex/issues/967
thread::Builder::new()
.stack_size(1 << 10)
.stack_size(16 << 10)
.spawn(run)
.unwrap()
.join()
Expand Down
5 changes: 4 additions & 1 deletion regex-syntax/src/hir/mod.rs
Expand Up @@ -2286,8 +2286,11 @@ mod tests {

// We run our test on a thread with a small stack size so we can
// force the issue more easily.
//
// NOTE(2023-03-21): See the corresponding test in 'crate::ast::tests'
// for context on the specific stack size chosen here.
thread::Builder::new()
.stack_size(1 << 10)
.stack_size(16 << 10)
.spawn(run)
.unwrap()
.join()
Expand Down

0 comments on commit eb7fb73

Please sign in to comment.