Skip to content

Commit

Permalink
ignore cache insertion error to avoid non-deterministic state
Browse files Browse the repository at this point in the history
  • Loading branch information
sh-cha committed Nov 1, 2024
1 parent 0a30558 commit 24501f7
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 26 deletions.
27 changes: 13 additions & 14 deletions crates/storage/src/module_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{hash::RandomState, num::NonZeroUsize, sync::Arc};
use bytes::Bytes;
use clru::{CLruCache, CLruCacheConfig};
use move_binary_format::{
errors::{Location, PartialVMError, VMError, VMResult},
errors::{Location, PartialVMError, VMResult},
CompiledModule,
};
use move_core_types::{language_storage::ModuleId, vm_status::StatusCode};
Expand All @@ -17,12 +17,6 @@ use crate::{
state_view::Checksum,
};

fn handle_cache_error(module_id: ModuleId) -> VMError {
PartialVMError::new(StatusCode::MEMORY_LIMIT_EXCEEDED)
.with_message("Module storage cache eviction error".to_string())
.finish(Location::Module(module_id))
}

/// Extension for modules stored in [InitialModuleCache] to also capture information about bytes
/// and hash.
#[derive(PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -97,7 +91,10 @@ impl InitiaModuleCache {
));
module_cache
.put_with_weight(key, ModuleWrapper::new(module, allocated_size))
.map_err(|_| handle_cache_error(module_id))?;
.unwrap_or_else(|_| {
eprintln!("WARNING: failed to insert module {:?} into cache; cache capacity might be too small", module_id.short_str_lossless().to_string());
None
});
Ok(())
}
}
Expand All @@ -121,7 +118,10 @@ impl InitiaModuleCache {
let module = Arc::new(ModuleCode::from_verified(verified_code, extension, version));
module_cache
.put_with_weight(key, ModuleWrapper::new(module.clone(), allocated_size))
.map_err(|_| handle_cache_error(module_id))?;
.unwrap_or_else(|_| {
eprintln!("WARNING: failed to insert module {:?} into cache; cache capacity might be too small", module_id.short_str_lossless().to_string());
None
});
Ok(module)
}
}
Expand Down Expand Up @@ -158,11 +158,10 @@ impl InitiaModuleCache {
let code_wrapper = ModuleWrapper::new(Arc::new(code), allocated_size);
module_cache
.put_with_weight(*checksum, code_wrapper.clone())
.map_err(|_| {
PartialVMError::new(StatusCode::MEMORY_LIMIT_EXCEEDED)
.with_message("Module storage cache eviction error".to_string())
.finish(Location::Module(id.clone()))
})?;
.unwrap_or_else(|_| {
eprintln!("WARNING: failed to insert module {:?} into cache; cache capacity might be too small", id.short_str_lossless().to_string());
None
});
Some(code_wrapper)
}
None => None,
Expand Down
21 changes: 9 additions & 12 deletions crates/storage/src/script_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ use std::{hash::RandomState, num::NonZeroUsize, sync::Arc};

use clru::{CLruCache, CLruCacheConfig};
use move_binary_format::{
errors::{Location, PartialVMError, VMResult},
errors::VMResult,
file_format::CompiledScript,
};
use move_core_types::vm_status::StatusCode;
use move_vm_runtime::Script;
use move_vm_types::code::Code;
use parking_lot::Mutex;
Expand Down Expand Up @@ -49,11 +48,10 @@ impl InitiaScriptCache {
// error occurs when the new script has a weight greater than the cache capacity
script_cache
.put_with_weight(key, ScriptWrapper::new(new_script, allocated_size))
.map_err(|_| {
PartialVMError::new(StatusCode::MEMORY_LIMIT_EXCEEDED)
.with_message("Script storage cache eviction error".to_string())
.finish(Location::Script)
})?;
.unwrap_or_else(|_| {
eprintln!("WARNING: failed to insert script into cache; cache capacity might be too small");
None
});

Ok(deserialized_script)
}
Expand Down Expand Up @@ -88,11 +86,10 @@ impl InitiaScriptCache {
if new_script.is_some() {
script_cache
.put_with_weight(key, ScriptWrapper::new(new_script.unwrap(), allocated_size))
.map_err(|_| {
PartialVMError::new(StatusCode::MEMORY_LIMIT_EXCEEDED)
.with_message("Script storage cache eviction error".to_string())
.finish(Location::Script)
})?;
.unwrap_or_else(|_| {
eprintln!("WARNING: failed to insert script into cache; cache capacity might be too small");
None
});
}
Ok(verified_script)
}
Expand Down

0 comments on commit 24501f7

Please sign in to comment.