Fix triggers being forgotten on after actions with on_exit: true

decrement_trigger do not delete triggers anymore when exiting

test still failing because filters start before start commands
This commit is contained in:
ppom 2025-08-08 12:00:00 +02:00
commit 58f4793308
No known key found for this signature in database
5 changed files with 90 additions and 45 deletions

2
TODO
View file

@ -4,3 +4,5 @@ stream: test regex ending with $
should an ipv6-mapped ipv4 match a pattern of type ipv6?
should it be normalized as ipv4 then?
fix filter commands executing before start commands

View file

@ -268,7 +268,7 @@ impl FilterManager {
let m = m.clone();
if exec_time <= now {
if state.decrement_trigger(&m, t) {
if state.decrement_trigger(&m, t, false) {
exec_now(&self.exec_limit, self.shutdown.clone(), action, m);
}
} else {
@ -289,7 +289,7 @@ impl FilterManager {
if !exiting || action.on_exit {
#[allow(clippy::unwrap_used)] // propagating panics is ok
let mut state = this.state.lock().unwrap();
if state.decrement_trigger(&m, t) {
if state.decrement_trigger(&m, t, exiting) {
exec_now(&this.exec_limit, this.shutdown, action, m);
}
}

View file

@ -140,7 +140,7 @@ impl State {
}
/// Returns whether we should still execute an action for this (Match, Time) trigger
pub fn decrement_trigger(&mut self, m: &Match, t: Time) -> bool {
pub fn decrement_trigger(&mut self, m: &Match, t: Time, exiting: bool) -> bool {
// We record triggered filters only when there is an action with an `after` directive
if self.has_after {
let mut exec_needed = false;
@ -153,16 +153,24 @@ impl State {
if let Some(count) = count {
exec_needed = true;
if count <= 1 {
self.triggers.fetch_update(mt.m, |map| {
map.and_then(|mut map| {
map.remove(&mt.t);
if map.is_empty() {
None
} else {
Some(map)
}
})
});
if !exiting {
self.triggers.fetch_update(mt.m, |map| {
map.and_then(|mut map| {
map.remove(&mt.t);
if map.is_empty() {
None
} else {
Some(map)
}
})
});
}
// else don't do anything
// Because that will remove the entry in the DB, and make
// it forget this trigger.
// Maybe we should have 2 maps for triggers:
// - The current for action counting, not persisted
// - Another like ordered_times, Tree<Time, Match>, persisted
} else {
self.triggers.fetch_update(mt.m, |map| {
map.map(|mut map| {
@ -429,7 +437,7 @@ mod tests {
assert!(state.triggers.tree().is_empty());
// Will be called immediately after, it returns true
assert!(state.decrement_trigger(&one, now));
assert!(state.decrement_trigger(&one, now, false));
}
#[tokio::test]
@ -484,22 +492,52 @@ mod tests {
&BTreeMap::from([(one.clone(), [(now, 3)].into())])
);
// Decrement → true
assert!(state.decrement_trigger(&one, now));
assert!(state.decrement_trigger(&one, now, false));
assert_eq!(
state.triggers.tree(),
&BTreeMap::from([(one.clone(), [(now, 2)].into())])
);
// Decrement → true
assert!(state.decrement_trigger(&one, now));
assert!(state.decrement_trigger(&one, now, false));
assert_eq!(
state.triggers.tree(),
&BTreeMap::from([(one.clone(), [(now, 1)].into())])
);
// Decrement → true
assert!(state.decrement_trigger(&one, now));
assert!(state.decrement_trigger(&one, now, false));
assert!(state.triggers.tree().is_empty());
// Decrement → false
assert!(!state.decrement_trigger(&one, now));
assert!(!state.decrement_trigger(&one, now, false));
// Add unique trigger (but decrement exiting-like)
state.add_trigger(one.clone(), now, None);
assert_eq!(
state.triggers.tree(),
&BTreeMap::from([(one.clone(), [(now, 3)].into())])
);
// Decrement → true
assert!(state.decrement_trigger(&one, now, true));
assert_eq!(
state.triggers.tree(),
&BTreeMap::from([(one.clone(), [(now, 2)].into())])
);
// Decrement → true
assert!(state.decrement_trigger(&one, now, true));
assert_eq!(
state.triggers.tree(),
&BTreeMap::from([(one.clone(), [(now, 1)].into())])
);
// Decrement but exiting → true, does nothing
assert!(state.decrement_trigger(&one, now, true));
assert_eq!(
state.triggers.tree(),
&BTreeMap::from([(one.clone(), [(now, 1)].into())])
);
// Decrement → true
assert!(state.decrement_trigger(&one, now, false));
assert!(state.triggers.tree().is_empty());
// Decrement → false
assert!(!state.decrement_trigger(&one, now, false));
// Add trigger with neighbour
state.add_trigger(one.clone(), now, None);
@ -509,25 +547,25 @@ mod tests {
&BTreeMap::from([(one.clone(), [(now_plus_1s, 3), (now, 3)].into())])
);
// Decrement → true
assert!(state.decrement_trigger(&one, now));
assert!(state.decrement_trigger(&one, now, false));
assert_eq!(
state.triggers.tree(),
&BTreeMap::from([(one.clone(), [(now_plus_1s, 3), (now, 2)].into())])
);
// Decrement → true
assert!(state.decrement_trigger(&one, now));
assert!(state.decrement_trigger(&one, now, false));
assert_eq!(
state.triggers.tree(),
&BTreeMap::from([(one.clone(), [(now_plus_1s, 3), (now, 1)].into())])
);
// Decrement → true
assert!(state.decrement_trigger(&one, now));
assert!(state.decrement_trigger(&one, now, false));
assert_eq!(
state.triggers.tree(),
&BTreeMap::from([(one.clone(), [(now_plus_1s, 3)].into())])
);
// Decrement → false
assert!(!state.decrement_trigger(&one, now));
assert!(!state.decrement_trigger(&one, now, false));
// Remove neighbour
state.remove_trigger(&one);
assert!(state.triggers.tree().is_empty());

View file

@ -25,6 +25,7 @@ local echo(message, before='true') = [
cmd: ['sh', '-c', 'seq 2 | while read i; do echo runtime $i; sleep 0.1; done'],
filters: {
f1: {
duplicate: 'rerun',
regex: [
'^runtime <num>$',
],
@ -33,8 +34,8 @@ local echo(message, before='true') = [
cmd: echo('runtime <num>'),
},
two: {
cmd: echo('after', before='sleep 1'),
after: '5s',
cmd: echo('after', before='sleep 0.2'),
after: '5m',
onexit: true,
},
},

View file

@ -5,7 +5,6 @@ use assert_fs::{prelude::*, TempDir};
use predicates::prelude::predicate;
#[test]
// #[ignore = "currently failing"] // FIXME
fn start_stop() {
let tmp_dir = assert_fs::TempDir::new().unwrap();
@ -27,27 +26,32 @@ fn start_stop() {
tmp_dir.child("log").assert(&output.join("\n"));
tmp_dir.child("log").write_str("").unwrap();
// // Second run
// run_reaction(&tmp_dir);
println!(
"DATABASE:\n{}",
std::fs::read_to_string(tmp_dir.child("reaction.db")).unwrap()
);
// // Expected output
// let output = [
// "start 1",
// "start 2",
// "runtime 1",
// "runtime 2",
// "runtime 1",
// "runtime 2",
// // no order required because they'll be awaken all together on exit
// "after",
// "after",
// "after",
// "after",
// "stop 1",
// "stop 2",
// "",
// ];
// tmp_dir.child("log").assert(&output.join("\n"));
// Second run
run_reaction(&tmp_dir);
// Expected output
let output = [
"start 1",
"start 2",
"runtime 1",
"runtime 2",
"runtime 1",
"runtime 2",
// no order required because they'll be awaken all together on exit
"after",
"after",
"after",
"after",
"stop 1",
"stop 2",
"",
];
tmp_dir.child("log").assert(&output.join("\n"));
}
fn run_reaction(tmp_dir: &TempDir) {