Skip to content

Commit

Permalink
fix: EXC-1889 Hook condition should be checked after every mgmt canis…
Browse files Browse the repository at this point in the history 
…ter call (#3988)

This PR ensures that the condition for a low-wasm memory hook will be
met after every mgmt canister call.
Since we already have tests that condition is checked after Updating
settings and Installing code, we will only add tests that snapshot
operations influence hook in the floow-up
https://github.com/dfinity/ic/pull/4023/files.
  • Loading branch information
@dragoljub-duric
dragoljub-duric authored Feb 19, 2025
1 parent 925e92a commit 0923aa8
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 148 deletions.
3 changes: 0 additions & 3 deletions rs/execution_environment/src/canister_manager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -643,9 +643,6 @@ impl CanisterManager {
if let Some(wasm_memory_limit) = settings.wasm_memory_limit() {
canister.system_state.wasm_memory_limit = Some(wasm_memory_limit);
}
// Change of `wasm_memory_limit` or `new_wasm_memory_threshold` or `memory_allocation`,
// can influence the satisfaction of the condition for `low_wasm_memory` hook.
canister.update_on_low_wasm_memory_hook_condition();
}

/// Tries to apply the requested settings on the canister identified by
Expand Down
154 changes: 62 additions & 92 deletions rs/execution_environment/src/canister_manager/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8128,118 +8128,88 @@ fn helper_update_settings_updates_hook_status(
updated_memory_state: MemoryState,
execute_hook_after_install: bool,
) {
with_setup(|canister_manager, mut state, subnet_id| {
let mut round_limits = RoundLimits {
instructions: as_round_instructions(EXECUTION_PARAMETERS.instruction_limits.message()),
subnet_available_memory: (*MAX_SUBNET_AVAILABLE_MEMORY),
subnet_available_callbacks: SUBNET_CALLBACK_SOFT_LIMIT as i64,
compute_allocation_used: state.total_compute_allocation(),
};
let mut test = ExecutionTestBuilder::new().build();

let wat = format!("(module (memory {used_wasm_memory_pages}))");
let mut initial_settings = CanisterSettingsArgsBuilder::new();

let wasm = wat::parse_str(wat).unwrap();
if let Some(MemoryAllocation::Reserved(memory_allocation)) =
initial_memory_state.memory_allocation
{
initial_settings = initial_settings.with_memory_allocation(memory_allocation.get());
}

let sender = canister_test_id(100).get();
if let Some(wasm_memory_limit) = initial_memory_state.wasm_memory_limit {
initial_settings = initial_settings.with_wasm_memory_limit(wasm_memory_limit.get());
}

let initial_settings = CanisterSettings {
wasm_memory_limit: initial_memory_state.wasm_memory_limit,
wasm_memory_threshold: initial_memory_state.wasm_memory_threshold,
memory_allocation: initial_memory_state.memory_allocation,
..Default::default()
};
if let Some(wasm_memory_threshold) = initial_memory_state.wasm_memory_threshold {
initial_settings = initial_settings.with_wasm_memory_threshold(wasm_memory_threshold.get());
}

let canister_id = canister_manager
.create_canister(
canister_change_origin_from_principal(&sender),
subnet_id,
*INITIAL_CYCLES,
initial_settings,
MAX_NUMBER_OF_CANISTERS,
&mut state,
SMALL_APP_SUBNET_MAX_SIZE,
&mut round_limits,
ResourceSaturation::default(),
&no_op_counter(),
)
.0
.unwrap();
let canister_id = test
.create_canister_with_settings(*INITIAL_CYCLES, initial_settings.build())
.unwrap();

let res = install_code(
&canister_manager,
InstallCodeContext {
origin: canister_change_origin_from_principal(&sender),
canister_id,
wasm_source: WasmSource::CanisterModule(CanisterModule::new(wasm)),
arg: vec![],
compute_allocation: None,
memory_allocation: None,
mode: CanisterInstallModeV2::Install,
},
&mut state,
&mut round_limits,
);
assert!(res.1.is_ok(), "{:?}", res.1);
let wat = format!("(module (memory {used_wasm_memory_pages}))");

let mut c_state = res.2.unwrap();
let wasm = wat::parse_str(wat).unwrap();

if execute_hook_after_install {
c_state.system_state.task_queue.pop_front();
}
test.install_canister(canister_id, wasm).unwrap();

assert_eq!(
c_state.system_state.task_queue.peek_hook_status(),
initial_memory_state.hook_status
);
if execute_hook_after_install {
test.canister_state_mut(canister_id)
.system_state
.task_queue
.pop_front();
}

state.put_canister_state(c_state);
assert_eq!(
test.canister_state_mut(canister_id)
.system_state
.task_queue
.peek_hook_status(),
initial_memory_state.hook_status
);

let mut settings = CanisterSettingsBuilder::new();
let mut settings = CanisterSettingsArgsBuilder::new();

if updated_memory_state.wasm_memory_threshold != initial_memory_state.wasm_memory_threshold
{
if let Some(updated_wasm_memory_threshold) = updated_memory_state.wasm_memory_threshold
{
settings = settings.with_wasm_memory_threshold(updated_wasm_memory_threshold);
}
if updated_memory_state.wasm_memory_threshold != initial_memory_state.wasm_memory_threshold {
if let Some(updated_wasm_memory_threshold) = updated_memory_state.wasm_memory_threshold {
settings = settings.with_wasm_memory_threshold(updated_wasm_memory_threshold.get());
}
}

if updated_memory_state.wasm_memory_limit != initial_memory_state.wasm_memory_limit {
if let Some(updated_wasm_memory_limit) = updated_memory_state.wasm_memory_limit {
settings = settings.with_wasm_memory_limit(updated_wasm_memory_limit);
}
if updated_memory_state.wasm_memory_limit != initial_memory_state.wasm_memory_limit {
if let Some(updated_wasm_memory_limit) = updated_memory_state.wasm_memory_limit {
settings = settings.with_wasm_memory_limit(updated_wasm_memory_limit.get());
}
}

if updated_memory_state.memory_allocation != initial_memory_state.memory_allocation {
if let Some(updated_memory_allocation) = updated_memory_state.memory_allocation {
settings = settings.with_memory_allocation(updated_memory_allocation);
}
if updated_memory_state.memory_allocation != initial_memory_state.memory_allocation {
if let Some(MemoryAllocation::Reserved(updated_memory_allocation)) =
updated_memory_state.memory_allocation
{
settings = settings.with_memory_allocation(updated_memory_allocation.get());
}
}

let canister = state.canister_state_mut(&canister_id).unwrap();
let payload = UpdateSettingsArgs {
canister_id: canister_id.get(),
settings: settings.build(),
sender_canister_version: None,
}
.encode();

assert!(canister_manager
.update_settings(
Time::from_nanos_since_unix_epoch(777),
canister_change_origin_from_principal(&sender),
settings.build(),
canister,
&mut round_limits,
ResourceSaturation::default(),
SMALL_APP_SUBNET_MAX_SIZE,
)
.is_ok());
test.subnet_message(Method::UpdateSettings, payload)
.unwrap();

assert_eq!(
state
.canister_state_mut(&canister_id)
.unwrap()
.system_state
.task_queue
.peek_hook_status(),
updated_memory_state.hook_status
);
})
assert_eq!(
test.canister_state_mut(canister_id)
.system_state
.task_queue
.peek_hook_status(),
updated_memory_state.hook_status
);
}

#[test]
Expand Down
Loading

0 comments on commit 0923aa8

Please sign in to comment.