From 42bac159ddad66957a64353d2806cbaf93ede4e4 Mon Sep 17 00:00:00 2001 From: Alexandre van Beurden Date: Tue, 12 Nov 2024 20:31:19 +0100 Subject: [PATCH] Fix timeout not correctly applied --- src/execution_profile.rs | 62 +++++++++++++++++-- .../convert/to_regex/transform.rs | 1 + src/fast_automaton/operation/intersection.rs | 8 ++- src/regex/operation/concat.rs | 52 ++++++++-------- 4 files changed, 90 insertions(+), 33 deletions(-) diff --git a/src/execution_profile.rs b/src/execution_profile.rs index 27c9296..2da9161 100644 --- a/src/execution_profile.rs +++ b/src/execution_profile.rs @@ -14,7 +14,7 @@ use crate::error::EngineError; /// execution_timeout: 1000, /// max_number_of_terms: 10, /// }; -/// +/// /// // Store the settings on the current thread. /// ThreadLocalParams::init_profile(&execution_profile); /// ``` @@ -90,7 +90,7 @@ impl ExecutionProfile { /// Assert that `execution_timeout` is not exceeded. /// /// Return empty if `execution_timeout` is not exceeded or if `start_execution_time` is not set. - /// + /// /// Return [`EngineError::OperationTimeOutError`] otherwise. pub fn assert_not_timed_out(&self) -> Result<(), EngineError> { if let Some(start) = self.start_execution_time { @@ -110,13 +110,12 @@ impl ExecutionProfile { } } - /// Hold [`ExecutionProfile`] on the current thread. -/// +/// /// The default [`ExecutionProfile`] is the following: /// ``` /// use regexsolver::execution_profile::ExecutionProfile; -/// +/// /// ExecutionProfile { /// max_number_of_states: 8192, /// start_execution_time: None, @@ -181,7 +180,7 @@ impl ThreadLocalParams { #[cfg(test)] mod tests { - use crate::regex::RegularExpression; + use crate::{regex::RegularExpression, Term}; use super::*; @@ -205,4 +204,55 @@ mod tests { Ok(()) } + + #[test] + fn test_execution_timeout_generate_strings() -> Result<(), String> { + let term = Term::from_regex(".*abc.*def.*qdsqd.*sqdsqd.*qsdsqdsqdz").unwrap(); + + let start_time = SystemTime::now(); + let execution_profile = ExecutionProfile { + max_number_of_states: 8192, + start_execution_time: Some(start_time), + execution_timeout: 100, + max_number_of_terms: 50, + }; + ThreadLocalParams::init_profile(&execution_profile); + + assert_eq!(EngineError::OperationTimeOutError, term.generate_strings(100).unwrap_err()); + + let run_duration = SystemTime::now() + .duration_since(start_time) + .expect("Time went backwards") + .as_millis(); + + println!("{run_duration}"); + assert!(run_duration <= execution_profile.execution_timeout + 50); + Ok(()) + } + + #[test] + fn test_execution_timeout_difference() -> Result<(), String> { + let term1 = Term::from_regex(".*abc.*def.*qdqd.*qsdsqdsqdz").unwrap(); + let term2 = Term::from_regex(".*abc.*def.*qdsqd.*sqdsqd.*qsdsqdsqdz.*abc.*def.*qdsqd.*sqdsqd.*qsdsqdsqdz.*abc.*def.*qdsqd.*sqdsqd.*qsdsqdsqdz").unwrap(); + + let start_time = SystemTime::now(); + let execution_profile = ExecutionProfile { + max_number_of_states: 8192, + start_execution_time: Some(start_time), + execution_timeout: 100, + max_number_of_terms: 50, + }; + ThreadLocalParams::init_profile(&execution_profile); + + assert_eq!(EngineError::OperationTimeOutError, term1.difference(&term2).unwrap_err()); + + let run_duration = SystemTime::now() + .duration_since(start_time) + .expect("Time went backwards") + .as_millis(); + + println!("{run_duration}"); + assert!(run_duration <= execution_profile.execution_timeout + 50); + Ok(()) + } } diff --git a/src/fast_automaton/convert/to_regex/transform.rs b/src/fast_automaton/convert/to_regex/transform.rs index 9fb2fd6..aaeca76 100644 --- a/src/fast_automaton/convert/to_regex/transform.rs +++ b/src/fast_automaton/convert/to_regex/transform.rs @@ -12,6 +12,7 @@ impl StateEliminationAutomaton { if self.cyclic { return self.convert_graph_to_regex(execution_profile); } + execution_profile.assert_not_timed_out()?; let mut regex_map: IntMap = IntMap::with_capacity_and_hasher( self.get_number_of_states(), diff --git a/src/fast_automaton/operation/intersection.rs b/src/fast_automaton/operation/intersection.rs index ab92e2f..c0eaf7a 100644 --- a/src/fast_automaton/operation/intersection.rs +++ b/src/fast_automaton/operation/intersection.rs @@ -1,4 +1,4 @@ -use crate::error::EngineError; +use crate::{error::EngineError, execution_profile::ThreadLocalParams}; use super::*; @@ -11,6 +11,8 @@ impl FastAutomaton { } else if other.is_total() { return Ok(self.clone()); } + let execution_profile = ThreadLocalParams::get_execution_profile(); + let new_spanning_set = self.spanning_set.merge(&other.spanning_set); let mut new_automaton = FastAutomaton::new_empty(); @@ -29,6 +31,7 @@ impl FastAutomaton { new_states.insert((self.start_state, other.start_state), initial_pair); while let Some(p) = worklist.pop_front() { + execution_profile.assert_not_timed_out()?; if self.accept_states.contains(&p.1) && other.accept_states.contains(&p.2) { new_automaton.accept(p.0); } @@ -67,6 +70,8 @@ impl FastAutomaton { } else if self.is_total() || other.is_total() { return Ok(true); } + let execution_profile = ThreadLocalParams::get_execution_profile(); + let new_spanning_set = self.spanning_set.merge(&other.spanning_set); let mut new_automaton = FastAutomaton::new_empty(); @@ -85,6 +90,7 @@ impl FastAutomaton { new_states.insert((self.start_state, other.start_state), initial_pair); while let Some(p) = worklist.pop_front() { + execution_profile.assert_not_timed_out()?; if self.accept_states.contains(&p.1) && other.accept_states.contains(&p.2) { return Ok(true); } diff --git a/src/regex/operation/concat.rs b/src/regex/operation/concat.rs index 38888c0..6907d9b 100644 --- a/src/regex/operation/concat.rs +++ b/src/regex/operation/concat.rs @@ -165,7 +165,32 @@ impl RegularExpression { this: &RegularExpression, that: &RegularExpression, ) -> Option { - if let ( + if this == that { + if let ( + RegularExpression::Repetition(this_regex, this_min, this_max_opt), + RegularExpression::Repetition(_, that_min, that_max_opt), + ) = (this, that) + { + let new_min = this_min + that_min; + let new_max_opt = + if let (Some(this_max), Some(that_max)) = (this_max_opt, that_max_opt) { + Some(this_max + that_max) + } else { + None + }; + Some(RegularExpression::Repetition( + this_regex.clone(), + new_min, + new_max_opt, + )) + } else { + Some(RegularExpression::Repetition( + Box::new(this.clone()), + 2, + Some(2), + )) + } + } else if let ( RegularExpression::Repetition(this_regex, this_min, this_max_opt), RegularExpression::Repetition(that_regex, that_min, that_max_opt), ) = (this, that) @@ -197,31 +222,6 @@ impl RegularExpression { } else { return None; } - } else if this == that { - if let ( - RegularExpression::Repetition(this_regex, this_min, this_max_opt), - RegularExpression::Repetition(_, that_min, that_max_opt), - ) = (this, that) - { - let new_min = this_min + that_min; - let new_max_opt = - if let (Some(this_max), Some(that_max)) = (this_max_opt, that_max_opt) { - Some(this_max + that_max) - } else { - None - }; - Some(RegularExpression::Repetition( - this_regex.clone(), - new_min, - new_max_opt, - )) - } else { - Some(RegularExpression::Repetition( - Box::new(this.clone()), - 2, - Some(2), - )) - } } else if let RegularExpression::Repetition(this_regex, this_min, this_max_opt) = this { if **this_regex == *that { let new_min = this_min + 1;