tremor-rs / tremor-runtime

Main Tremor Project Rust Codebase
https://www.tremor.rs
Apache License 2.0
852 stars 125 forks source link

New module system enters infinite loop with selfuse in module. #152

Closed m0n0chr0m3 closed 4 years ago

m0n0chr0m3 commented 4 years ago

Problem When a module (in the experimental/pre-release version of tremor 0.8, on branch https://github.com/wayfair-tremor/tremor-runtime/tree/spike-modularity-combined) uses itself, the current approach fails to detect this, and overflows the Rust runtime stack.

Steps A proof-of-concept is available in f539136913a4a15e38eccc14168b1e0bf2cb5762.

Licenser commented 4 years ago

Good catch! That's a awesome find :) thank you!

Licenser commented 4 years ago

Thinking about this, I suspect we'll run into the same issue with cyclic use.

darach commented 4 years ago

Can confirm the cyclic case - adding a failing pp_cyclic test for it.

darach commented 4 years ago

Interestingly, when run as a test we get SIGABRT:

thread 'pp_cyclic' has overflowed its stack
fatal runtime error: stack overflow
error: test failed, to rerun pass '-p tremor-runtime --test script_error'

Caused by:
  process didn't exit successfully: `/Code/oss/wayfair-tremor/tremor-runtime/target/debug/deps/script_error-3cebb02a3463fbc1` (signal: 6, SIGABRT: process abort signal)
darach commented 4 years ago

Cyclic errors are now detected and reported:

Top level ( self ):

Error:
      | ^ Cyclic dependency detected: script.tremor -> ./script.tremor -> ./script.tremor

Module path nested ( mutual ):

Error:
      | ^ Cyclic dependency detected: script.tremor -> ./foo/bar.tremor -> ./foo/baz.tremor -> ./foo/bar.tremor
darach commented 4 years ago

@m0n0chr0m3 Hopefully this covers all the cases you discovered? Let me know if so and we can close the bug!

m0n0chr0m3 commented 4 years ago

I think it does, yes. Thanks.

(Closing the issue, since the fix is merged into the branch to which this issue applies.)