Garbage collection does not work with certain types stored in the Engines

[JoshuaBatty]

I've just opened up a draft PR #5813 with a new test called garbage_collection_storage that is able to reproduce this. Running the test produces the following stack trace.

running 1 test
thread 'thread 'thread '<unnamed><unnamed><unnamed>' panicked at ' panicked at ' panicked at sway-core/src/concurrent_slab.rssway-core/src/concurrent_slab.rssway-core/src/concurrent_slab.rs:90:14:
no entry found for key
:90:14:
no entry found for key
:90:14:
stack backtrace:
no entry found for key
   0: rust_begin_unwind
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/std/src/panicking.rs:647:5
   1: core::panicking::panic_fmt
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/panicking.rs:72:14
   2: core::panicking::panic_display
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/panicking.rs:196:5
   3: core::panicking::panic_str
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/panicking.rs:171:5
   4: core::option::expect_failed
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/option.rs:1988:5
   5: core::option::Option<T>::expect
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/option.rs:894:21
   6: <std::collections::hash::map::HashMap<K,V,S> as core::ops::index::Index<&Q>>::index
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/std/src/collections/hash/map.rs:1341:23
   7: sway_core::concurrent_slab::ConcurrentSlab<T>::get
             at /Users/josh/Documents/rust/fuel/sway/sway-core/src/concurrent_slab.rs:90:14
   8: <sway_core::decl_engine::parsed_engine::ParsedDeclEngine as sway_core::decl_engine::parsed_engine::ParsedDeclEngineGet<sway_core::decl_engine::parsed_id::ParsedDeclId<sway_core::language::parsed::declaration::impl_trait::ImplTrait>,sway_core::language::parsed::declaration::impl_trait::ImplTrait>>::get
             at /Users/josh/Documents/rust/fuel/sway/sway-core/src/decl_engine/parsed_engine.rs:59:17
   9: sway_core::decl_engine::parsed_engine::ParsedDeclEngine::get_impl_trait
             at /Users/josh/Documents/rust/fuel/sway/sway-core/src/decl_engine/parsed_engine.rs:220:9
  10: sway_lsp::traverse::parsed_tree::<impl sway_lsp::traverse::Parse for sway_core::decl_engine::parsed_id::ParsedDeclId<sway_core::language::parsed::declaration::impl_trait::ImplTrait>>::parse
             at ./src/traverse/parsed_tree.rs:772:26
  11: sway_lsp::traverse::parsed_tree::<impl sway_lsp::traverse::Parse for sway_core::language::parsed::declaration::Declaration>::parse
             at ./src/traverse/parsed_tree.rs:127:48
  12: sway_lsp::traverse::parsed_tree::<impl sway_lsp::traverse::Parse for sway_core::language::parsed::AstNode>::parse
             at ./src/traverse/parsed_tree.rs:108:57
  13: sway_lsp::traverse::parsed_tree::ParsedTree::traverse_node
             at ./src/traverse/parsed_tree.rs:53:9
  14: sway_lsp::core::session::traverse::{{closure}}
             at ./src/core/session.rs:424:59
  15: sway_lsp::core::session::parse_ast_to_tokens::{{closure}}
             at ./src/core/session.rs:506:35
  16: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &F>::call_mut
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/ops/function.rs:272:13
  17: <core::slice::iter::Iter<T> as core::iter::traits::iterator::Iterator>::for_each
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/slice/iter/macros.rs:254:21
  18: <rayon::iter::for_each::ForEachConsumer<F> as rayon::iter::plumbing::Folder<T>>::consume_iter
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.10.0/src/iter/for_each.rs:55:9
  19: rayon::iter::plumbing::Producer::fold_with
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.10.0/src/iter/plumbing/mod.rs:109:9
  20: rayon::iter::plumbing::bridge_producer_consumer::helper
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.10.0/src/iter/plumbing/mod.rs:437:13
  21: rayon::iter::plumbing::bridge_producer_consumer::helper::{{closure}}
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.10.0/src/iter/plumbing/mod.rs:426:21
  22: rayon_core::join::join_context::call_b::{{closure}}
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/join/mod.rs:129:25
  23: rayon_core::job::JobResult<T>::call::{{closure}}
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/job.rs:218:41
  24: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/panic/unwind_safe.rs:272:9
  25: std::panicking::try::do_call
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/std/src/panicking.rs:554:40
  26: ___rust_try
  27: std::panicking::try
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/std/src/panicking.rs:518:19
  28: std::panic::catch_unwind
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/std/src/panic.rs:142:14
  29: rayon_core::unwind::halt_unwinding
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/unwind.rs:17:5
  30: rayon_core::job::JobResult<T>::call
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/job.rs:218:15
  31: <rayon_core::job::StackJob<L,F,R> as rayon_core::job::Job>::execute
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/job.rs:120:32
  32: rayon_core::job::JobRef::execute
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/job.rs:64:9
  33: rayon_core::registry::WorkerThread::execute
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/registry.rs:860:9
  34: rayon_core::registry::WorkerThread::wait_until_cold
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/registry.rs:794:21
  35: rayon_core::registry::WorkerThread::wait_until
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/registry.rs:769:13
  36: rayon_core::registry::WorkerThread::wait_until_out_of_work
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/registry.rs:818:9
  37: rayon_core::registry::main_loop
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/registry.rs:923:5
  38: rayon_core::registry::ThreadBuilder::run
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/registry.rs:53:18
  39: <rayon_core::registry::DefaultSpawn as rayon_core::registry::ThreadSpawn>::spawn::{{closure}}
             at /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-core-1.12.1/src/registry.rs:98:20

[JoshuaBatty]

Ok I have been able to reproduce this error with the new test consistently within 1-5 did_change events at this commit 348d008

I just rebased onto master and have done 8 successful local runs

1 time it produced the same crash as above, the other 7 times it now handles 600 did_change events with GC on every keystroke without a problem.

Running this test in CI seems to fail everytime still....

[JoshuaBatty]

Just leaving some more info. It seems that this error occurs when compilation is started and completed successfully, but the TyDecl::type_check function never inserts a TyStorageDecl for some reason.

Compiling project...
Compilation failed ❌
🗑️  Garbage collecting 🗑️
Compiling project...
Inserting StorageDecl into decl_engine 🤝
Traversing project...
Compilation successful ✅
version: 62
version: 63
🗑️  Garbage collecting 🗑️
Compiling project...
Compilation failed ❌
version: 64
🗑️  Garbage collecting 🗑️
Compiling project...
Inserting StorageDecl into decl_engine 🤝
Traversing project...
Compilation successful ✅
🗑️  Garbage collecting 🗑️
Compiling project...
Traversing project...
thread 'thread '<unnamed>' panicked at sway-core/src/concurrent_slab.rs:90:14:
no entry found for key
<unnamed>' panicked at sway-core/src/concurrent_slab.rs:90:14thread ':
no entry found for key
stack backtrace:
<unnamed>' panicked at sway-core/src/concurrent_slab.rs:90:14:
no entry found for key
   0: rust_begin_unwind
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/std/src/panicking.rs:647:5
   1: core::panicking::panic_fmt
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/panicking.rs:72:14
   2: core::panicking::panic_display
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/panicking.rs:196:5
   3: core::panicking::panic_str
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/panicking.rs:171:5
   4: core::option::expect_failed
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/option.rs:1988:5
   5: core::option::Option<T>::expect
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/option.rs:894:21
   6: <std::collections::hash::map::HashMap<K,V,S> as core::ops::index::Index<&Q>>::index
             at /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/std/src/collections/hash/map.rs:1341:23
   7: sway_core::concurrent_slab::ConcurrentSlab<T>::get
             at /Users/josh/Documents/rust/fuel/sway/sway-core/src/concurrent_slab.rs:90:14
   8: <sway_core::decl_engine::parsed_engine::ParsedDeclEngine as sway_core::decl_engine::parsed_engine::ParsedDeclEngineGet<sway_core::decl_engine::parsed_id::ParsedDeclId<sway_core::language::parsed::declaration::storage::StorageDeclaration>,sway_core::language::parsed::declaration::storage::StorageDeclaration>>::get
             at /Users/josh/Documents/rust/fuel/sway/sway-core/src/decl_engine/parsed_engine.rs:59:17
   9: sway_core::decl_engine::parsed_engine::ParsedDeclEngine::get_storage

[JoshuaBatty]

Ok I think i'm narrowing in on this. Compilation is returning an old cached AST.

let cache_up_to_date = entry.modified_time == modified_time || {
    let src = std::fs::read_to_string(path.as_path()).unwrap();

    let mut hasher = DefaultHasher::new();
    src.hash(&mut hasher);
    let hash = hasher.finish();

    hash == entry.hash
};

In this instance, we hit the || and load the file from the path into a string and compare. Now, because we actually write into this file from the LSP, the compiler may be have been trigger by a did_change event, and during this the LSP has actually saved a newer copy at this location. This would explain why I originally found this error while rapidly triggering between enter and backspace events.

[JoshuaBatty]

Ok i've got a fix up now at #5813

turned out to be 3 things

1. don't load the string from file as above, instead hash the input that was used to kick off compilation
2. check if the compiler retuned a cached AST for the users workspace, if so, we traverse the AST's with the current LSP version of the engines as it hasn't had garbage collection applied.
3. Also do the above check in the compilation thread. Only mem swap the engines_clone and engines if a cached AST for the users workspace was not used.