uhop / node-re2

node.js bindings for RE2: fast, safe alternative to backtracking regular expression engines.
Other
489 stars 53 forks source link

Breaking Bug: v1.17.8 is broken in worker threads (pointer being freed was not allocated) #143

Closed titanism closed 10 months ago

titanism commented 1 year ago

This patch version released has broken our builds. Specifically we're using ava (which uses worker threads under the hood). When we set workerThreads: false it works fine, however when we use ava normal default behavior we get a pointer error (see below).

Reference: https://github.com/avajs/ava/issues/3145

node(1813,0x17272f000) malloc: *** error for object 0x6000010b83a0: pointer being freed was not allocated
node(1813,0x17272f000) malloc: *** set a breakpoint in malloc_error_break to debug
[1]    1812 abort      npx ava --serial --fail-fast --verbose

Downgrading to v1.17.7 fixes the issue.

Might have stemmed from one of these commits?

Ping @uhop

Thanks as always 🙏

uhop commented 1 year ago

1.17.8 has no code changes. But the following related dependencies were updated:

Either the dependencies introduced a bug or a new toolchain exposed a latent bug in my/re2 code. :-(

uhop commented 1 year ago

re2 library, which is used as a dependency, had no changes and was not updated for 1.17.8.

uhop commented 1 year ago

@titanism could you create a minimal repro case? It’ll help to debug the problem.

uhop commented 1 year ago

@titanism another important question: what platform did fail? Just Windows? Anything else?

titanism commented 1 year ago

@uhop I've tested 1.18.0 and the same bug persists. To answer your questions - the platform is macOS Monterey on Apple M1 - I haven't tested others yet. To reproduce, I've tried to just create some test cases with ava and re2, but I haven't been able to get a reproduction outside of our project. That being said, the project is fully open source on GitHub, and you can easily reproduce it as follows:

  1. git clone https://github.com/forwardemail/forwardemail.net.git fe-test
  2. cd fe-test
  3. Edit package.json and change re2 from 1.17.7 to 1.17.8
  4. npm install
  5. npm test
uhop commented 1 year ago

That being said, the project is fully open source on GitHub, and you can easily reproduce it as follows

Let me guess: it requires multiple servers to configure and run and contains mounds of non-trivial code. ;-)

Before I started to dig inside: is it possible that you to allocate an RE2 object in one thread and free it in the other?

Hmm, doing so should be valid. Allocating/freeing across process boundaries is not valid. Unless you run different V8 engines per thread.

titanism commented 1 year ago

I'm not sure how the internals work, but if you set workerThreads option in ava to false as mentioned in https://github.com/avajs/ava/issues/3145 the bug doesn't occur. Something changed in v1.17.8 that caused this, but I haven't pinpointed it yet - not sure where to start to debug. Definitely something related to processes/threads/worker threads though (since that's obvious from https://github.com/avajs/ava/issues/3145).

uhop commented 10 months ago

I probably should post the comment here instead of the closed issue. Anyway this is the link: https://github.com/uhop/node-re2/issues/73#issuecomment-1781984355

titanism commented 10 months ago

I haven't seen this issue come up again, and we're already on Node v18 and v20, so assuming it's gone. If it happens again I'll let you know. Thank you for trying to reproduce @uhop.