foundry-rs / compilers

Utilities for working with native solc and compiling projects.
Apache License 2.0
58 stars 36 forks source link

Help Needed - Making use of the`Solc` module gives `ExecutableFileBusy` error #113

Closed TilakMaddy closed 2 months ago

TilakMaddy commented 2 months ago
called `Result::unwrap()` on an `Err` value: SvmError(IoError(Os { code: 26, kind: ExecutableFileBusy, message: "Text file busy" }))

We have a function that looks like this

pub fn load_solidity_source_unit(filepath: &str) -> WorkspaceContext {
    let solidity_file = &ensure_valid_solidity_file(filepath);
    let solidity_content = std::fs::read_to_string(solidity_file).unwrap();

    let compiler_input = CompilerInput::new(solidity_file.as_path()).unwrap();
    let compiler_input = compiler_input.first().unwrap(); // There's only 1 type of file in the path (solidity)

    let version = Solc::detect_version(&Source {
        content: Arc::new(solidity_content.clone()),
    })
    .unwrap(); // <--------- Above shown SvmError happens here when unwrapping
    .....
}

Note that this function is used in tests, therefore 2 or more may run in parallel but I see that there are relevant file locks all over the code so I am having a hard time understanding why we're getting this error

More context: https://github.com/Cyfrin/aderyn/pull/333

Happy to go into detail if it helps!

🙏

UPDATE

Full function can be found here

https://github.com/Cyfrin/aderyn/blob/internal-compilation-framework/aderyn_core/src/detect/test_utils/load_source_unit.rs

mattsse commented 2 months ago

there are likely still a few race conditions when two processes try to install the same solc binary and use it.

to unblock you, you could use the serial-test crate in CI

TilakMaddy commented 2 months ago

Yes, just to be extra safe I even tried using serial tests in CI

And there are only 2 tests that make use of this function - still fails.

Example test function

    #[test]
    #[serial(fc_solc)] // <---- `serial_test` crate
    fn test_siblings_by_loading_contract_directly() {
        let _lock = take_loader_lock();
        let context =
            load_solidity_source_unit("../tests/contract-playground/src/StorageConditionals.sol"); <----Function shown in PR desc
       ........
    }
DaniPopes commented 2 months ago

are you using cargo-nextest? If so, tests are ran in parallel regardless of serial_test

TilakMaddy commented 2 months ago

No we're not using Cargo-nextest

TilakMaddy commented 2 months ago

If I only have 1 test, it passes. So for now, I have removed the other test. But we need to bring it back at some point in order to scale these tests. (we plan to have ~400 to ~900 detectors and each one would have it's test suite). So any advice/guidance would help in which direction to proceed

DaniPopes commented 2 months ago

Well I'd recommend you do with a few retries to ignore these spurious failures and improves testing speed.

I believe this error happens because an executable is downloading/extracting in one thread and another will see that the path exists, try to execute it and fail.

DaniPopes commented 2 months ago

Also just a suggestion, I would look into how clippy manages hundreds of lints if you plan having that many detectors.

TilakMaddy commented 2 months ago

Here's another thing I forgot to mention - the tests (even in parallel) work properly when I do cargo test in my local Macbook Pro but fails in CI

TilakMaddy commented 2 months ago

Thanks 🙏 everyone. The tests are stable now although they are run in serial order. As @DaniPopes said, it was spurious errors and needed more tries.

I'm closing the issue for now