From bb5c7909a32d5bd2f504fc8814d9d9a891e066b2 Mon Sep 17 00:00:00 2001 From: Ilya Denisov Date: Fri, 23 May 2025 23:33:15 +0300 Subject: [PATCH 1/8] Don't use global variable for timers queue --- crates/javy/src/apis/timers/mod.rs | 579 ++++++++++++++--------------- crates/javy/src/runtime.rs | 41 +- 2 files changed, 295 insertions(+), 325 deletions(-) diff --git a/crates/javy/src/apis/timers/mod.rs b/crates/javy/src/apis/timers/mod.rs index f824d7f8..be8c436d 100644 --- a/crates/javy/src/apis/timers/mod.rs +++ b/crates/javy/src/apis/timers/mod.rs @@ -61,10 +61,10 @@ impl TimerQueue { .duration_since(UNIX_EPOCH) .unwrap() .as_millis() as u64; - + let id = self.next_id; self.next_id += 1; - + let timer = Timer { id, // For delay=0, set fire_time to current time to ensure immediate availability @@ -72,7 +72,7 @@ impl TimerQueue { callback, interval_ms: None, }; - + self.timers.push(timer); id } @@ -82,10 +82,10 @@ impl TimerQueue { .duration_since(UNIX_EPOCH) .unwrap() .as_millis() as u64; - + let id = self.next_id; self.next_id += 1; - + let timer = Timer { id, // For delay=0, set fire_time to current time to ensure immediate availability @@ -93,7 +93,7 @@ impl TimerQueue { callback, interval_ms: Some(delay_ms), }; - + self.timers.push(timer); id } @@ -104,14 +104,14 @@ impl TimerQueue { .duration_since(UNIX_EPOCH) .unwrap() .as_millis() as u64; - + let new_timer = Timer { id: timer.id, fire_time: if interval_ms == 0 { now } else { now + interval_ms as u64 }, callback: timer.callback, interval_ms: timer.interval_ms, }; - + self.timers.push(new_timer); } } @@ -127,9 +127,9 @@ impl TimerQueue { .duration_since(UNIX_EPOCH) .unwrap() .as_millis() as u64; - + let mut expired = Vec::new(); - + while let Some(timer) = self.timers.peek() { if timer.fire_time <= now { expired.push(self.timers.pop().unwrap()); @@ -137,7 +137,7 @@ impl TimerQueue { break; } } - + expired } @@ -146,67 +146,109 @@ impl TimerQueue { } } -static TIMER_QUEUE: OnceLock>> = OnceLock::new(); - -fn get_timer_queue() -> &'static Arc> { - TIMER_QUEUE.get_or_init(|| Arc::new(Mutex::new(TimerQueue::new()))) +pub struct TimersRuntime { + queue: Arc>, } -/// Register timer functions on the global object -pub(crate) fn register(this: Ctx<'_>) -> Result<()> { - let globals = this.globals(); - - globals.set( - "setTimeout", - Function::new( - this.clone(), - MutFn::new(move |cx, args| { - let (cx, args) = hold_and_release!(cx, args); - set_timeout(hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) - }), - )?, - )?; - - globals.set( - "clearTimeout", - Function::new( - this.clone(), - MutFn::new(move |cx, args| { - let (cx, args) = hold_and_release!(cx, args); - clear_timeout(hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) - }), - )?, - )?; - - globals.set( - "setInterval", - Function::new( - this.clone(), - MutFn::new(move |cx, args| { - let (cx, args) = hold_and_release!(cx, args); - set_interval(hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) - }), - )?, - )?; - - globals.set( - "clearInterval", - Function::new( - this.clone(), - MutFn::new(move |cx, args| { - let (cx, args) = hold_and_release!(cx, args); - clear_interval(hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) - }), - )?, - )?; - - Ok(()) +impl TimersRuntime { + pub fn new() -> Self { + Self { + queue: Arc::new(Mutex::new(TimerQueue::new())), + } + } + + /// Register timer functions on the global object + pub fn register_globals(&self, this: Ctx<'_>) -> Result<()> { + let globals = this.globals(); + + let queue = self.queue.clone(); + globals.set( + "setTimeout", + Function::new( + this.clone(), + MutFn::new(move |cx, args| { + let (cx, args) = hold_and_release!(cx, args); + set_timeout(queue.clone(),hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) + }), + )?, + )?; + + let queue = self.queue.clone(); + globals.set( + "clearTimeout", + Function::new( + this.clone(), + MutFn::new(move |cx, args| { + let (cx, args) = hold_and_release!(cx, args); + clear_timeout(queue.clone(), hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) + }), + )?, + )?; + + let queue = self.queue.clone(); + globals.set( + "setInterval", + Function::new( + this.clone(), + MutFn::new(move |cx, args| { + let (cx, args) = hold_and_release!(cx, args); + set_interval(queue.clone(), hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) + }), + )?, + )?; + + let queue = self.queue.clone(); + globals.set( + "clearInterval", + Function::new( + this.clone(), + MutFn::new(move |cx, args| { + let (cx, args) = hold_and_release!(cx, args); + clear_interval(queue.clone(), hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) + }), + )?, + )?; + + Ok(()) + } + + /// Process expired timers - should be called by the event loop + pub fn process_timers(&self, ctx: Ctx<'_>) -> Result<()> { + let mut queue = self.queue.lock().unwrap(); + let expired_timers = queue.get_expired_timers(); + + // Separate intervals that need rescheduling + let (intervals, timeouts): (Vec<_>, Vec<_>) = expired_timers.into_iter() + .partition(|timer| timer.interval_ms.is_some()); + + // Reschedule intervals before releasing the lock + for interval in &intervals { + queue.reschedule_interval(interval.clone()); + } + + drop(queue); // Release lock before executing JavaScript + + // Execute all timer callbacks (both timeouts and intervals) + for timer in timeouts.into_iter().chain(intervals.into_iter()) { + if let Err(e) = ctx.eval::<(), _>(timer.callback.as_str()) { + eprintln!("Timer callback error: {}", e); + } + } + + Ok(()) + } + + /// Check if there are pending timers + pub fn has_pending_timers(&self) -> bool { + let queue = self.queue.lock().unwrap(); + queue.has_pending_timers() + } } -fn set_timeout<'js>(args: Args<'js>) -> Result> { +fn set_timeout<'js>(queue: Arc>, args: Args<'js>) -> Result> { let (ctx, args) = args.release(); let args = args.into_inner(); - + if args.is_empty() { return Err(anyhow!("setTimeout requires at least 1 argument")); } @@ -227,32 +269,34 @@ fn set_timeout<'js>(args: Args<'js>) -> Result> { 0 }; - let mut queue = get_timer_queue().lock().unwrap(); + let mut queue = queue.lock().unwrap(); let timer_id = queue.add_timer(delay_ms, callback_str); - + drop(queue); + Ok(Value::new_int(ctx, timer_id as i32)) } -fn clear_timeout<'js>(args: Args<'js>) -> Result> { +fn clear_timeout<'js>(queue: Arc>, args: Args<'js>) -> Result> { let (ctx, args) = args.release(); let args = args.into_inner(); - + if args.is_empty() { return Ok(Value::new_undefined(ctx)); } let timer_id = args[0].as_number().unwrap_or(0.0) as u32; - - let mut queue = get_timer_queue().lock().unwrap(); + + let mut queue = queue.lock().unwrap(); queue.remove_timer(timer_id); - + drop(queue); + Ok(Value::new_undefined(ctx)) } -fn set_interval<'js>(args: Args<'js>) -> Result> { +fn set_interval<'js>(queue: Arc>, args: Args<'js>) -> Result> { let (ctx, args) = args.release(); let args = args.into_inner(); - + if args.is_empty() { return Err(anyhow!("setInterval requires at least 1 argument")); } @@ -273,58 +317,28 @@ fn set_interval<'js>(args: Args<'js>) -> Result> { 0 }; - let mut queue = get_timer_queue().lock().unwrap(); + let mut queue = queue.lock().unwrap(); let timer_id = queue.add_interval(interval_ms, callback_str); - + drop(queue); + Ok(Value::new_int(ctx, timer_id as i32)) } -fn clear_interval<'js>(args: Args<'js>) -> Result> { +fn clear_interval<'js>(queue: Arc>, args: Args<'js>) -> Result> { let (ctx, args) = args.release(); let args = args.into_inner(); - + if args.is_empty() { return Ok(Value::new_undefined(ctx)); } let timer_id = args[0].as_number().unwrap_or(0.0) as u32; - - let mut queue = get_timer_queue().lock().unwrap(); - queue.remove_timer(timer_id); - - Ok(Value::new_undefined(ctx)) -} -/// Process expired timers - should be called by the event loop -pub fn process_timers(ctx: Ctx<'_>) -> Result<()> { - let mut queue = get_timer_queue().lock().unwrap(); - let expired_timers = queue.get_expired_timers(); - - // Separate intervals that need rescheduling - let (intervals, timeouts): (Vec<_>, Vec<_>) = expired_timers.into_iter() - .partition(|timer| timer.interval_ms.is_some()); - - // Reschedule intervals before releasing the lock - for interval in &intervals { - queue.reschedule_interval(interval.clone()); - } - - drop(queue); // Release lock before executing JavaScript - - // Execute all timer callbacks (both timeouts and intervals) - for timer in timeouts.into_iter().chain(intervals.into_iter()) { - if let Err(e) = ctx.eval::<(), _>(timer.callback.as_str()) { - eprintln!("Timer callback error: {}", e); - } - } - - Ok(()) -} + let mut queue = queue.lock().unwrap(); + queue.remove_timer(timer_id); + drop(queue); -/// Check if there are pending timers -pub fn has_pending_timers() -> bool { - let queue = get_timer_queue().lock().unwrap(); - queue.has_pending_timers() + Ok(Value::new_undefined(ctx)) } #[cfg(test)] @@ -336,22 +350,22 @@ mod tests { #[test] fn test_timer_queue() { let mut queue = TimerQueue::new(); - + // Add some timers let id1 = queue.add_timer(100, "console.log('timer1')".to_string()); let id2 = queue.add_timer(50, "console.log('timer2')".to_string()); let id3 = queue.add_timer(200, "console.log('timer3')".to_string()); - + assert_eq!(id1, 1); assert_eq!(id2, 2); assert_eq!(id3, 3); - + assert!(queue.has_pending_timers()); - + // Remove a timer assert!(queue.remove_timer(id2)); assert!(!queue.remove_timer(999)); // Non-existent timer - + assert!(queue.has_pending_timers()); } @@ -361,18 +375,16 @@ mod tests { config.timers(true); let runtime = Runtime::new(config)?; runtime.context().with(|cx| { - register(cx.clone())?; - // Check that setTimeout is available let result: Value = cx.eval("typeof setTimeout")?; let type_str = val_to_string(&cx, result)?; assert_eq!(type_str, "function"); - + // Check that clearTimeout is available let result: Value = cx.eval("typeof clearTimeout")?; let type_str = val_to_string(&cx, result)?; assert_eq!(type_str, "function"); - + Ok::<_, Error>(()) })?; Ok(()) @@ -384,18 +396,16 @@ mod tests { config.timers(true); let runtime = Runtime::new(config)?; runtime.context().with(|cx| { - register(cx.clone())?; - // Test setTimeout with string callback let result: Value = cx.eval("setTimeout('1+1', 100)")?; let timer_id = result.as_number().unwrap() as i32; assert!(timer_id > 0); - - // Test setTimeout with function callback + + // Test setTimeout with function callback let result: Value = cx.eval("setTimeout(function() { return 42; }, 50)")?; let timer_id2 = result.as_number().unwrap() as i32; assert!(timer_id2 > timer_id); - + Ok::<_, Error>(()) })?; Ok(()) @@ -407,13 +417,11 @@ mod tests { config.timers(true); let runtime = Runtime::new(config)?; runtime.context().with(|cx| { - register(cx.clone())?; - // Create a timer and clear it let result: Value = cx.eval("const id = setTimeout('console.log(\"test\")', 1000); clearTimeout(id); id")?; let timer_id = result.as_number().unwrap() as i32; assert!(timer_id > 0); - + Ok::<_, Error>(()) })?; Ok(()) @@ -421,66 +429,61 @@ mod tests { #[test] fn test_timer_execution() -> Result<()> { - // Clear any existing timers from other tests - { - let mut queue = get_timer_queue().lock().unwrap(); - queue.timers.clear(); - } - let mut config = Config::default(); config.timers(true); let runtime = Runtime::new(config)?; + + // Use a unique variable name to avoid interference between tests + let unique_var = format!("timerExecuted_{}", std::process::id()); + let timer_code = format!("globalThis.{} = false; setTimeout('globalThis.{} = true', 0)", unique_var, unique_var); + runtime.context().with(|cx| { - register(cx.clone())?; - - // Use a unique variable name to avoid interference between tests - let unique_var = format!("timerExecuted_{}", std::process::id()); - let timer_code = format!("globalThis.{} = false; setTimeout('globalThis.{} = true', 0)", unique_var, unique_var); cx.eval::<(), _>(timer_code.as_str())?; - - // Process timers immediately without sleep - they should be available - process_timers(cx.clone())?; - + Ok::<_, Error>(()) + })?; + + // Process timers immediately without sleep - they should be available + runtime.resolve_pending_jobs()?; + + runtime.context().with(|cx| { // Check if timer was executed let check_code = format!("globalThis.{}", unique_var); let result: Value = cx.eval(check_code.as_str())?; - + assert!(result.as_bool().unwrap_or(false)); - + Ok::<_, Error>(()) })?; + Ok(()) } #[test] fn test_timer_with_delay() -> Result<()> { - // Clear any existing timers from other tests - { - let mut queue = get_timer_queue().lock().unwrap(); - queue.timers.clear(); - } - let mut config = Config::default(); config.timers(true); let runtime = Runtime::new(config)?; + + // Use unique variable name to avoid interference between tests + let unique_var = format!("delayedTimer_{}", std::process::id()); + + // Set a timer with a delay that shouldn't fire immediately + let timer_code = format!("globalThis.{} = false; setTimeout('globalThis.{} = true', 1000)", unique_var, unique_var); + runtime.context().with(|cx| { - register(cx.clone())?; - - // Use unique variable name to avoid interference between tests - let unique_var = format!("delayedTimer_{}", std::process::id()); - - // Set a timer with a delay that shouldn't fire immediately - let timer_code = format!("globalThis.{} = false; setTimeout('globalThis.{} = true', 1000)", unique_var, unique_var); cx.eval::<(), _>(timer_code.as_str())?; - - // Process timers immediately - should not execute - process_timers(cx.clone())?; - + Ok::<_, Error>(()) + })?; + + // Process timers immediately - should not execute + runtime.resolve_pending_jobs()?; + + runtime.context().with(|cx| { // Check if timer was NOT executed let check_code = format!("globalThis.{}", unique_var); let result: Value = cx.eval(check_code.as_str())?; assert!(!result.as_bool().unwrap_or(true)); - + Ok::<_, Error>(()) })?; Ok(()) @@ -488,23 +491,16 @@ mod tests { #[test] fn test_multiple_timers() -> Result<()> { - // Clear any existing timers from other tests - { - let mut queue = get_timer_queue().lock().unwrap(); - queue.timers.clear(); - } - let mut config = Config::default(); config.timers(true); let runtime = Runtime::new(config)?; + + // Use unique variable names to avoid interference between tests + let unique_id = std::process::id(); + let timer1_var = format!("timer1_{}", unique_id); + let timer2_var = format!("timer2_{}", unique_id); + runtime.context().with(|cx| { - register(cx.clone())?; - - // Use unique variable names to avoid interference between tests - let unique_id = std::process::id(); - let timer1_var = format!("timer1_{}", unique_id); - let timer2_var = format!("timer2_{}", unique_id); - // Set multiple timers let timer_code = format!(" globalThis.{} = false; @@ -513,10 +509,14 @@ mod tests { setTimeout('globalThis.{} = true', 0); ", timer1_var, timer2_var, timer1_var, timer2_var); cx.eval::<(), _>(timer_code.as_str())?; - - // Process timers - process_timers(cx.clone())?; - + Ok::<_, Error>(()) + })?; + + + // Process timers + runtime.resolve_pending_jobs()?; + + runtime.context().with(|cx| { // Check if both timers were executed let check1_code = format!("globalThis.{}", timer1_var); let check2_code = format!("globalThis.{}", timer2_var); @@ -524,7 +524,7 @@ mod tests { let result2: Value = cx.eval(check2_code.as_str())?; assert!(result1.as_bool().unwrap_or(false)); assert!(result2.as_bool().unwrap_or(false)); - + Ok::<_, Error>(()) })?; Ok(()) @@ -532,37 +532,34 @@ mod tests { #[test] fn test_clear_timeout_removes_timer() -> Result<()> { - // Clear any existing timers from other tests - { - let mut queue = get_timer_queue().lock().unwrap(); - queue.timers.clear(); - } - let mut config = Config::default(); config.timers(true); let runtime = Runtime::new(config)?; + + // Use unique variable name to avoid interference between tests + let unique_var = format!("clearedTimer_{}", std::process::id()); + + // Set a timer and immediately clear it + let timer_code = format!(" + globalThis.{} = false; + const id = setTimeout('globalThis.{} = true', 0); + clearTimeout(id); + ", unique_var, unique_var); + runtime.context().with(|cx| { - register(cx.clone())?; - - // Use unique variable name to avoid interference between tests - let unique_var = format!("clearedTimer_{}", std::process::id()); - - // Set a timer and immediately clear it - let timer_code = format!(" - globalThis.{} = false; - const id = setTimeout('globalThis.{} = true', 0); - clearTimeout(id); - ", unique_var, unique_var); cx.eval::<(), _>(timer_code.as_str())?; - - // Process timers - process_timers(cx.clone())?; - + Ok::<_, Error>(()) + })?; + + // Process timers + runtime.resolve_pending_jobs()?; + + runtime.context().with(|cx| { // Check if timer was NOT executed let check_code = format!("globalThis.{}", unique_var); let result: Value = cx.eval(check_code.as_str())?; assert!(!result.as_bool().unwrap_or(true)); - + Ok::<_, Error>(()) })?; Ok(()) @@ -570,27 +567,20 @@ mod tests { #[test] fn test_has_pending_timers() -> Result<()> { - // Clear any existing timers from other tests and verify clean state - { - let mut queue = get_timer_queue().lock().unwrap(); - queue.timers.clear(); - } - - // Initially no pending timers - assert!(!has_pending_timers()); - let mut config = Config::default(); config.timers(true); let runtime = Runtime::new(config)?; + + // Initially no pending timers + assert!(!runtime.has_pending_timers()); + runtime.context().with(|cx| { - register(cx.clone())?; - // Add a timer cx.eval::<(), _>("setTimeout('console.log(\"test\")', 1000)")?; - + // Should have pending timers - assert!(has_pending_timers()); - + assert!(runtime.has_pending_timers()); + Ok::<_, Error>(()) })?; Ok(()) @@ -598,26 +588,18 @@ mod tests { #[test] fn test_set_interval_basic() -> Result<()> { - // Clear any existing timers from other tests - { - let mut queue = get_timer_queue().lock().unwrap(); - queue.timers.clear(); - } - let mut config = Config::default(); config.timers(true); let runtime = Runtime::new(config)?; runtime.context().with(|cx| { - register(cx.clone())?; - // Test setInterval with string callback let result: Value = cx.eval("setInterval('1+1', 100)")?; let interval_id = result.as_number().unwrap() as i32; assert!(interval_id > 0); - + // Should have pending timers - assert!(has_pending_timers()); - + assert!(runtime.has_pending_timers()); + Ok::<_, Error>(()) })?; Ok(()) @@ -625,23 +607,15 @@ mod tests { #[test] fn test_clear_interval() -> Result<()> { - // Clear any existing timers from other tests - { - let mut queue = get_timer_queue().lock().unwrap(); - queue.timers.clear(); - } - let mut config = Config::default(); config.timers(true); let runtime = Runtime::new(config)?; runtime.context().with(|cx| { - register(cx.clone())?; - // Create an interval and clear it let result: Value = cx.eval("const id = setInterval('console.log(\"test\")', 1000); clearInterval(id); id")?; let interval_id = result.as_number().unwrap() as i32; assert!(interval_id > 0); - + Ok::<_, Error>(()) })?; Ok(()) @@ -649,36 +623,33 @@ mod tests { #[test] fn test_interval_execution_and_rescheduling() -> Result<()> { - // Clear any existing timers from other tests - { - let mut queue = get_timer_queue().lock().unwrap(); - queue.timers.clear(); - } - let mut config = Config::default(); config.timers(true); let runtime = Runtime::new(config)?; + + // Use unique variable name to avoid interference between tests + let unique_var = format!("intervalCount_{}", std::process::id()); + let interval_code = format!("globalThis.{} = 0; setInterval('globalThis.{}++', 0)", unique_var, unique_var); + runtime.context().with(|cx| { - register(cx.clone())?; - - // Use unique variable name to avoid interference between tests - let unique_var = format!("intervalCount_{}", std::process::id()); - let interval_code = format!("globalThis.{} = 0; setInterval('globalThis.{}++', 0)", unique_var, unique_var); cx.eval::<(), _>(interval_code.as_str())?; - - // Process timers multiple times to test rescheduling - process_timers(cx.clone())?; - process_timers(cx.clone())?; - process_timers(cx.clone())?; - + Ok::<_, Error>(()) + })?; + + // Process timers multiple times to test rescheduling + runtime.resolve_pending_jobs()?; + runtime.resolve_pending_jobs()?; + runtime.resolve_pending_jobs()?; + + runtime.context().with(|cx| { // Check if interval executed multiple times let check_code = format!("globalThis.{}", unique_var); let result: Value = cx.eval(check_code.as_str())?; let count = result.as_number().unwrap_or(0.0) as i32; - + // Should have executed at least twice (showing it's repeating) assert!(count >= 2, "Interval should have executed multiple times, got {}", count); - + Ok::<_, Error>(()) })?; Ok(()) @@ -686,40 +657,37 @@ mod tests { #[test] fn test_clear_interval_stops_repetition() -> Result<()> { - // Clear any existing timers from other tests - { - let mut queue = get_timer_queue().lock().unwrap(); - queue.timers.clear(); - } - let mut config = Config::default(); config.timers(true); let runtime = Runtime::new(config)?; + + // Use unique variable name to avoid interference between tests + let unique_var = format!("clearedIntervalCount_{}", std::process::id()); + runtime.context().with(|cx| { - register(cx.clone())?; - - // Use unique variable name to avoid interference between tests - let unique_var = format!("clearedIntervalCount_{}", std::process::id()); let interval_code = format!(" globalThis.{} = 0; const id = setInterval('globalThis.{}++', 0); clearInterval(id); ", unique_var, unique_var); cx.eval::<(), _>(interval_code.as_str())?; - - // Process timers multiple times - process_timers(cx.clone())?; - process_timers(cx.clone())?; - process_timers(cx.clone())?; - + Ok::<_, Error>(()) + })?; + + // Process timers multiple times + runtime.resolve_pending_jobs()?; + runtime.resolve_pending_jobs()?; + runtime.resolve_pending_jobs()?; + + runtime.context().with(|cx| { // Check that interval was NOT executed let check_code = format!("globalThis.{}", unique_var); let result: Value = cx.eval(check_code.as_str())?; let count = result.as_number().unwrap_or(-1.0) as i32; - + // Should still be 0 (not executed) assert_eq!(count, 0, "Cleared interval should not execute, got {}", count); - + Ok::<_, Error>(()) })?; Ok(()) @@ -727,23 +695,16 @@ mod tests { #[test] fn test_interval_and_timeout_coexistence() -> Result<()> { - // Clear any existing timers from other tests - { - let mut queue = get_timer_queue().lock().unwrap(); - queue.timers.clear(); - } - let mut config = Config::default(); config.timers(true); let runtime = Runtime::new(config)?; + + // Use unique variable names + let unique_id = std::process::id(); + let timeout_var = format!("timeoutExecuted_{}", unique_id); + let interval_var = format!("intervalCount_{}", unique_id); + runtime.context().with(|cx| { - register(cx.clone())?; - - // Use unique variable names - let unique_id = std::process::id(); - let timeout_var = format!("timeoutExecuted_{}", unique_id); - let interval_var = format!("intervalCount_{}", unique_id); - // Set timeout first, then interval - both with 0 delay let timer_code = format!(" globalThis.{} = false; @@ -752,31 +713,39 @@ mod tests { setInterval('globalThis.{}++', 0); ", timeout_var, interval_var, timeout_var, interval_var); cx.eval::<(), _>(timer_code.as_str())?; - - // Process timers first time - should execute both timeout and first interval - process_timers(cx.clone())?; - - // Check both timeout and interval results - let timeout_check = format!("globalThis.{}", timeout_var); - let interval_check = format!("globalThis.{}", interval_var); - + Ok::<_, Error>(()) + })?; + + // Process timers first time - should execute both timeout and first interval + runtime.resolve_pending_jobs()?; + + // Check both timeout and interval results + let timeout_check = format!("globalThis.{}", timeout_var); + let interval_check = format!("globalThis.{}", interval_var); + + runtime.context().with(|cx| { let timeout_result: Value = cx.eval(timeout_check.as_str())?; let interval_result: Value = cx.eval(interval_check.as_str())?; - + assert!(timeout_result.as_bool().unwrap_or(false), "Timeout should have executed"); assert!(interval_result.as_number().unwrap_or(0.0) as i32 >= 1, "Interval should have executed at least once"); - - // Process timers again to verify interval repeats (timeout shouldn't run again) - process_timers(cx.clone())?; + + Ok::<_, Error>(()) + })?; + + // Process timers again to verify interval repeats (timeout shouldn't run again) + runtime.resolve_pending_jobs()?; + + runtime.context().with(|cx| { let interval_result2: Value = cx.eval(interval_check.as_str())?; let timeout_result2: Value = cx.eval(timeout_check.as_str())?; - + // Timeout should still be true (unchanged), interval should have incremented assert!(timeout_result2.as_bool().unwrap_or(false), "Timeout should remain executed"); assert!(interval_result2.as_number().unwrap_or(0.0) as i32 >= 2, "Interval should have executed multiple times"); - + Ok::<_, Error>(()) })?; Ok(()) } -} \ No newline at end of file +} \ No newline at end of file diff --git a/crates/javy/src/runtime.rs b/crates/javy/src/runtime.rs index 3dbb0a04..2edd0faa 100644 --- a/crates/javy/src/runtime.rs +++ b/crates/javy/src/runtime.rs @@ -3,7 +3,7 @@ use super::from_js_error; #[cfg(feature = "json")] use crate::apis::json; use crate::{ - apis::{base64, console, random, stream_io, text_encoding, timers}, + apis::{base64, console, random, stream_io, text_encoding, timers::TimersRuntime}, config::{JSIntrinsics, JavyIntrinsics}, Config, }; @@ -40,20 +40,24 @@ pub struct Runtime { // Read above on the usage of `ManuallyDrop`. inner: ManuallyDrop, /// Whether timers are enabled - timers_enabled: bool, + timers: Option, } impl Runtime { /// Creates a new [Runtime]. pub fn new(config: Config) -> Result { let rt = ManuallyDrop::new(QRuntime::new()?); - let timers_enabled = config.intrinsics.contains(JSIntrinsics::TIMERS); + let timers = if config.intrinsics.contains(JSIntrinsics::TIMERS) { + Some(TimersRuntime::new()) + } else { + None + }; - let context = Self::build_from_config(&rt, config)?; - Ok(Self { inner: rt, context, timers_enabled }) + let context = Self::build_from_config(&rt, config, &timers)?; + Ok(Self { inner: rt, context, timers }) } - fn build_from_config(rt: &QRuntime, cfg: Config) -> Result> { + fn build_from_config(rt: &QRuntime, cfg: Config, timers: &Option) -> Result> { let cfg = cfg.validate()?; let intrinsics = &cfg.intrinsics; let javy_intrinsics = &cfg.javy_intrinsics; @@ -156,8 +160,8 @@ impl Runtime { .expect("registering StreamIO functions to succeed"); } - if intrinsics.contains(JSIntrinsics::TIMERS) { - timers::register(ctx.clone()) + if let Some(timers) = timers { + timers.register_globals(ctx.clone()) .expect("registering timer APIs to succeed"); } }); @@ -174,8 +178,8 @@ impl Runtime { pub fn resolve_pending_jobs(&self) -> Result<()> { // Process timers if enabled self.context.with(|ctx| { - if self.has_timers_enabled() { - timers::process_timers(ctx.clone()).unwrap_or_else(|e| { + if let Some(timers) = &self.timers { + timers.process_timers(ctx.clone()).unwrap_or_else(|e| { eprintln!("Timer processing error: {}", e); }); } @@ -200,18 +204,15 @@ impl Runtime { /// Returns true if there are pending jobs in the queue. pub fn has_pending_jobs(&self) -> bool { let has_js_jobs = self.inner.is_job_pending(); - let has_timer_jobs = if self.has_timers_enabled() { - timers::has_pending_timers() - } else { - false - }; - - has_js_jobs || has_timer_jobs + has_js_jobs || self.has_pending_timers() } - /// Returns true if timers are enabled for this runtime. - pub fn has_timers_enabled(&self) -> bool { - self.timers_enabled + pub fn has_pending_timers(&self) -> bool { + if let Some(timers) = &self.timers { + timers.has_pending_timers() + } else { + false + } } /// Compiles the given module to bytecode. From 3a45f9c794803da361c86f098a68a70e6e7912f1 Mon Sep 17 00:00:00 2001 From: Ilya Denisov Date: Sat, 24 May 2025 01:47:39 +0300 Subject: [PATCH 2/8] Simplify code a little --- crates/javy/src/apis/timers/mod.rs | 380 +++++++++++++++-------------- 1 file changed, 196 insertions(+), 184 deletions(-) diff --git a/crates/javy/src/apis/timers/mod.rs b/crates/javy/src/apis/timers/mod.rs index be8c436d..5e2b8df1 100644 --- a/crates/javy/src/apis/timers/mod.rs +++ b/crates/javy/src/apis/timers/mod.rs @@ -1,6 +1,6 @@ use std::{ collections::BinaryHeap, - sync::{Arc, Mutex, OnceLock}, + sync::{Arc, Mutex}, time::{SystemTime, UNIX_EPOCH}, }; @@ -11,141 +11,6 @@ use crate::{ }; use anyhow::{anyhow, Result}; -/// Timer entry in the timer queue -#[derive(Debug, Clone)] -struct Timer { - id: u32, - fire_time: u64, // milliseconds since UNIX epoch - callback: String, // JavaScript code to execute - interval_ms: Option, // If Some(), this is a repeating timer -} - -impl PartialEq for Timer { - fn eq(&self, other: &Self) -> bool { - self.fire_time == other.fire_time - } -} - -impl Eq for Timer {} - -impl PartialOrd for Timer { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for Timer { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - // Reverse order for min-heap behavior - other.fire_time.cmp(&self.fire_time) - } -} - -/// Global timer queue -#[derive(Debug)] -struct TimerQueue { - timers: BinaryHeap, - next_id: u32, -} - -impl TimerQueue { - fn new() -> Self { - Self { - timers: BinaryHeap::new(), - next_id: 1, - } - } - - fn add_timer(&mut self, delay_ms: u32, callback: String) -> u32 { - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_millis() as u64; - - let id = self.next_id; - self.next_id += 1; - - let timer = Timer { - id, - // For delay=0, set fire_time to current time to ensure immediate availability - fire_time: if delay_ms == 0 { now } else { now + delay_ms as u64 }, - callback, - interval_ms: None, - }; - - self.timers.push(timer); - id - } - - fn add_interval(&mut self, delay_ms: u32, callback: String) -> u32 { - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_millis() as u64; - - let id = self.next_id; - self.next_id += 1; - - let timer = Timer { - id, - // For delay=0, set fire_time to current time to ensure immediate availability - fire_time: if delay_ms == 0 { now } else { now + delay_ms as u64 }, - callback, - interval_ms: Some(delay_ms), - }; - - self.timers.push(timer); - id - } - - fn reschedule_interval(&mut self, timer: Timer) { - if let Some(interval_ms) = timer.interval_ms { - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_millis() as u64; - - let new_timer = Timer { - id: timer.id, - fire_time: if interval_ms == 0 { now } else { now + interval_ms as u64 }, - callback: timer.callback, - interval_ms: timer.interval_ms, - }; - - self.timers.push(new_timer); - } - } - - fn remove_timer(&mut self, timer_id: u32) -> bool { - let original_len = self.timers.len(); - self.timers.retain(|timer| timer.id != timer_id); - self.timers.len() != original_len - } - - fn get_expired_timers(&mut self) -> Vec { - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_millis() as u64; - - let mut expired = Vec::new(); - - while let Some(timer) = self.timers.peek() { - if timer.fire_time <= now { - expired.push(self.timers.pop().unwrap()); - } else { - break; - } - } - - expired - } - - fn has_pending_timers(&self) -> bool { - !self.timers.is_empty() - } -} - pub struct TimersRuntime { queue: Arc>, } @@ -168,7 +33,8 @@ impl TimersRuntime { this.clone(), MutFn::new(move |cx, args| { let (cx, args) = hold_and_release!(cx, args); - set_timeout(queue.clone(),hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) + set_timeout(&queue, hold!(cx.clone(), args)) + .map_err(|e| to_js_error(cx, e)) }), )?, )?; @@ -180,7 +46,8 @@ impl TimersRuntime { this.clone(), MutFn::new(move |cx, args| { let (cx, args) = hold_and_release!(cx, args); - clear_timeout(queue.clone(), hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) + clear_timeout(&queue, hold!(cx.clone(), args)) + .map_err(|e| to_js_error(cx, e)) }), )?, )?; @@ -192,7 +59,8 @@ impl TimersRuntime { this.clone(), MutFn::new(move |cx, args| { let (cx, args) = hold_and_release!(cx, args); - set_interval(queue.clone(), hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) + set_interval(&queue, hold!(cx.clone(), args)) + .map_err(|e| to_js_error(cx, e)) }), )?, )?; @@ -204,7 +72,8 @@ impl TimersRuntime { this.clone(), MutFn::new(move |cx, args| { let (cx, args) = hold_and_release!(cx, args); - clear_interval(queue.clone(), hold!(cx.clone(), args)).map_err(|e| to_js_error(cx, e)) + clear_interval(&queue, hold!(cx.clone(), args)) + .map_err(|e| to_js_error(cx, e)) }), )?, )?; @@ -217,19 +86,17 @@ impl TimersRuntime { let mut queue = self.queue.lock().unwrap(); let expired_timers = queue.get_expired_timers(); - // Separate intervals that need rescheduling - let (intervals, timeouts): (Vec<_>, Vec<_>) = expired_timers.into_iter() - .partition(|timer| timer.interval_ms.is_some()); - // Reschedule intervals before releasing the lock - for interval in &intervals { - queue.reschedule_interval(interval.clone()); + for timer in &expired_timers { + if let Some(interval_ms) = timer.interval_ms { + queue.add_timer(interval_ms, true, timer.callback.clone(), Some(timer.id)); + } } drop(queue); // Release lock before executing JavaScript // Execute all timer callbacks (both timeouts and intervals) - for timer in timeouts.into_iter().chain(intervals.into_iter()) { + for timer in &expired_timers { if let Err(e) = ctx.eval::<(), _>(timer.callback.as_str()) { eprintln!("Timer callback error: {}", e); } @@ -245,7 +112,110 @@ impl TimersRuntime { } } -fn set_timeout<'js>(queue: Arc>, args: Args<'js>) -> Result> { +/// Timer entry in the timer queue +#[derive(Debug, Clone)] +struct Timer { + id: u32, + fire_time: u64, // milliseconds since UNIX epoch + callback: String, // JavaScript code to execute + interval_ms: Option, // If Some(), this is a repeating timer +} + +impl PartialEq for Timer { + fn eq(&self, other: &Self) -> bool { + self.fire_time == other.fire_time + } +} + +impl Eq for Timer {} + +impl PartialOrd for Timer { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Timer { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + // Reverse order for min-heap behavior + other.fire_time.cmp(&self.fire_time) + } +} + +/// Global timer queue +#[derive(Debug)] +struct TimerQueue { + timers: BinaryHeap, + next_id: u32, +} + +impl TimerQueue { + fn new() -> Self { + Self { + timers: BinaryHeap::new(), + next_id: 1, + } + } + + fn add_timer( + &mut self, + delay_ms: u32, + repeat: bool, + callback: String, + reuse_id: Option, + ) -> u32 { + let now = Self::now(); + + let id = reuse_id.unwrap_or_else(|| { + let id = self.next_id; + self.next_id += 1; + id + }); + + let timer = Timer { + id, + fire_time: now + delay_ms as u64, + callback, + interval_ms: if repeat { Some(delay_ms) } else { None }, + }; + + self.timers.push(timer); + id + } + + fn remove_timer(&mut self, timer_id: u32) -> bool { + let original_len = self.timers.len(); + self.timers.retain(|timer| timer.id != timer_id); + self.timers.len() != original_len + } + + fn get_expired_timers(&mut self) -> Vec { + let now = Self::now(); + let mut expired = Vec::new(); + while let Some(timer) = self.timers.peek() { + if timer.fire_time <= now { + expired.push(self.timers.pop().unwrap()); + } else { + break; + } + } + + expired + } + + fn has_pending_timers(&self) -> bool { + !self.timers.is_empty() + } + + fn now() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_millis() as u64 + } +} + +fn set_timeout<'js>(queue: &Arc>, args: Args<'js>) -> Result> { let (ctx, args) = args.release(); let args = args.into_inner(); @@ -270,13 +240,13 @@ fn set_timeout<'js>(queue: Arc>, args: Args<'js>) -> Result(queue: Arc>, args: Args<'js>) -> Result> { +fn clear_timeout<'js>(queue: &Arc>, args: Args<'js>) -> Result> { let (ctx, args) = args.release(); let args = args.into_inner(); @@ -293,7 +263,7 @@ fn clear_timeout<'js>(queue: Arc>, args: Args<'js>) -> Result< Ok(Value::new_undefined(ctx)) } -fn set_interval<'js>(queue: Arc>, args: Args<'js>) -> Result> { +fn set_interval<'js>(queue: &Arc>, args: Args<'js>) -> Result> { let (ctx, args) = args.release(); let args = args.into_inner(); @@ -318,13 +288,13 @@ fn set_interval<'js>(queue: Arc>, args: Args<'js>) -> Result(queue: Arc>, args: Args<'js>) -> Result> { +fn clear_interval<'js>(queue: &Arc>, args: Args<'js>) -> Result> { let (ctx, args) = args.release(); let args = args.into_inner(); @@ -352,9 +322,9 @@ mod tests { let mut queue = TimerQueue::new(); // Add some timers - let id1 = queue.add_timer(100, "console.log('timer1')".to_string()); - let id2 = queue.add_timer(50, "console.log('timer2')".to_string()); - let id3 = queue.add_timer(200, "console.log('timer3')".to_string()); + let id1 = queue.add_timer(100, false, "console.log('timer1')".to_string(), None); + let id2 = queue.add_timer(50, false, "console.log('timer2')".to_string(), None); + let id3 = queue.add_timer(200, false, "console.log('timer3')".to_string(), None); assert_eq!(id1, 1); assert_eq!(id2, 2); @@ -418,7 +388,9 @@ mod tests { let runtime = Runtime::new(config)?; runtime.context().with(|cx| { // Create a timer and clear it - let result: Value = cx.eval("const id = setTimeout('console.log(\"test\")', 1000); clearTimeout(id); id")?; + let result: Value = cx.eval( + "const id = setTimeout('console.log(\"test\")', 1000); clearTimeout(id); id", + )?; let timer_id = result.as_number().unwrap() as i32; assert!(timer_id > 0); @@ -435,9 +407,12 @@ mod tests { // Use a unique variable name to avoid interference between tests let unique_var = format!("timerExecuted_{}", std::process::id()); - let timer_code = format!("globalThis.{} = false; setTimeout('globalThis.{} = true', 0)", unique_var, unique_var); runtime.context().with(|cx| { + let timer_code = format!( + "globalThis.{} = false; setTimeout('globalThis.{} = true', 0)", + unique_var, unique_var + ); cx.eval::<(), _>(timer_code.as_str())?; Ok::<_, Error>(()) })?; @@ -467,10 +442,12 @@ mod tests { // Use unique variable name to avoid interference between tests let unique_var = format!("delayedTimer_{}", std::process::id()); - // Set a timer with a delay that shouldn't fire immediately - let timer_code = format!("globalThis.{} = false; setTimeout('globalThis.{} = true', 1000)", unique_var, unique_var); - runtime.context().with(|cx| { + // Set a timer with a delay that shouldn't fire immediately + let timer_code = format!( + "globalThis.{} = false; setTimeout('globalThis.{} = true', 1000)", + unique_var, unique_var + ); cx.eval::<(), _>(timer_code.as_str())?; Ok::<_, Error>(()) })?; @@ -502,17 +479,19 @@ mod tests { runtime.context().with(|cx| { // Set multiple timers - let timer_code = format!(" + let timer_code = format!( + " globalThis.{} = false; globalThis.{} = false; setTimeout('globalThis.{} = true', 0); setTimeout('globalThis.{} = true', 0); - ", timer1_var, timer2_var, timer1_var, timer2_var); + ", + timer1_var, timer2_var, timer1_var, timer2_var + ); cx.eval::<(), _>(timer_code.as_str())?; Ok::<_, Error>(()) })?; - // Process timers runtime.resolve_pending_jobs()?; @@ -539,14 +518,16 @@ mod tests { // Use unique variable name to avoid interference between tests let unique_var = format!("clearedTimer_{}", std::process::id()); - // Set a timer and immediately clear it - let timer_code = format!(" - globalThis.{} = false; - const id = setTimeout('globalThis.{} = true', 0); - clearTimeout(id); - ", unique_var, unique_var); - runtime.context().with(|cx| { + // Set a timer and immediately clear it + let timer_code = format!( + " + globalThis.{} = false; + const id = setTimeout('globalThis.{} = true', 0); + clearTimeout(id); + ", + unique_var, unique_var + ); cx.eval::<(), _>(timer_code.as_str())?; Ok::<_, Error>(()) })?; @@ -612,7 +593,9 @@ mod tests { let runtime = Runtime::new(config)?; runtime.context().with(|cx| { // Create an interval and clear it - let result: Value = cx.eval("const id = setInterval('console.log(\"test\")', 1000); clearInterval(id); id")?; + let result: Value = cx.eval( + "const id = setInterval('console.log(\"test\")', 1000); clearInterval(id); id", + )?; let interval_id = result.as_number().unwrap() as i32; assert!(interval_id > 0); @@ -629,9 +612,12 @@ mod tests { // Use unique variable name to avoid interference between tests let unique_var = format!("intervalCount_{}", std::process::id()); - let interval_code = format!("globalThis.{} = 0; setInterval('globalThis.{}++', 0)", unique_var, unique_var); runtime.context().with(|cx| { + let interval_code = format!( + "globalThis.{} = 0; setInterval('globalThis.{}++', 0)", + unique_var, unique_var + ); cx.eval::<(), _>(interval_code.as_str())?; Ok::<_, Error>(()) })?; @@ -648,7 +634,11 @@ mod tests { let count = result.as_number().unwrap_or(0.0) as i32; // Should have executed at least twice (showing it's repeating) - assert!(count >= 2, "Interval should have executed multiple times, got {}", count); + assert!( + count >= 2, + "Interval should have executed multiple times, got {}", + count + ); Ok::<_, Error>(()) })?; @@ -665,11 +655,14 @@ mod tests { let unique_var = format!("clearedIntervalCount_{}", std::process::id()); runtime.context().with(|cx| { - let interval_code = format!(" + let interval_code = format!( + " globalThis.{} = 0; const id = setInterval('globalThis.{}++', 0); clearInterval(id); - ", unique_var, unique_var); + ", + unique_var, unique_var + ); cx.eval::<(), _>(interval_code.as_str())?; Ok::<_, Error>(()) })?; @@ -686,7 +679,11 @@ mod tests { let count = result.as_number().unwrap_or(-1.0) as i32; // Should still be 0 (not executed) - assert_eq!(count, 0, "Cleared interval should not execute, got {}", count); + assert_eq!( + count, 0, + "Cleared interval should not execute, got {}", + count + ); Ok::<_, Error>(()) })?; @@ -706,12 +703,15 @@ mod tests { runtime.context().with(|cx| { // Set timeout first, then interval - both with 0 delay - let timer_code = format!(" + let timer_code = format!( + " globalThis.{} = false; globalThis.{} = 0; setTimeout('globalThis.{} = true', 0); setInterval('globalThis.{}++', 0); - ", timeout_var, interval_var, timeout_var, interval_var); + ", + timeout_var, interval_var, timeout_var, interval_var + ); cx.eval::<(), _>(timer_code.as_str())?; Ok::<_, Error>(()) })?; @@ -727,8 +727,14 @@ mod tests { let timeout_result: Value = cx.eval(timeout_check.as_str())?; let interval_result: Value = cx.eval(interval_check.as_str())?; - assert!(timeout_result.as_bool().unwrap_or(false), "Timeout should have executed"); - assert!(interval_result.as_number().unwrap_or(0.0) as i32 >= 1, "Interval should have executed at least once"); + assert!( + timeout_result.as_bool().unwrap_or(false), + "Timeout should have executed" + ); + assert!( + interval_result.as_number().unwrap_or(0.0) as i32 >= 1, + "Interval should have executed at least once" + ); Ok::<_, Error>(()) })?; @@ -741,11 +747,17 @@ mod tests { let timeout_result2: Value = cx.eval(timeout_check.as_str())?; // Timeout should still be true (unchanged), interval should have incremented - assert!(timeout_result2.as_bool().unwrap_or(false), "Timeout should remain executed"); - assert!(interval_result2.as_number().unwrap_or(0.0) as i32 >= 2, "Interval should have executed multiple times"); + assert!( + timeout_result2.as_bool().unwrap_or(false), + "Timeout should remain executed" + ); + assert!( + interval_result2.as_number().unwrap_or(0.0) as i32 >= 2, + "Interval should have executed multiple times" + ); Ok::<_, Error>(()) })?; Ok(()) } -} \ No newline at end of file +} From 283c75bcd507717cacc7943c243a0eb43b908bcf Mon Sep 17 00:00:00 2001 From: Ilya Denisov Date: Sat, 24 May 2025 02:02:10 +0300 Subject: [PATCH 3/8] Separate JS bindings from queue implementation --- crates/javy/src/apis/timers/mod.rs | 134 +-------------------------- crates/javy/src/apis/timers/queue.rs | 134 +++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 130 deletions(-) create mode 100644 crates/javy/src/apis/timers/queue.rs diff --git a/crates/javy/src/apis/timers/mod.rs b/crates/javy/src/apis/timers/mod.rs index 5e2b8df1..aa158cd3 100644 --- a/crates/javy/src/apis/timers/mod.rs +++ b/crates/javy/src/apis/timers/mod.rs @@ -1,8 +1,7 @@ -use std::{ - collections::BinaryHeap, - sync::{Arc, Mutex}, - time::{SystemTime, UNIX_EPOCH}, -}; +use std::sync::{Arc, Mutex}; + +mod queue; +use queue::TimerQueue; use crate::{ hold, hold_and_release, @@ -112,109 +111,6 @@ impl TimersRuntime { } } -/// Timer entry in the timer queue -#[derive(Debug, Clone)] -struct Timer { - id: u32, - fire_time: u64, // milliseconds since UNIX epoch - callback: String, // JavaScript code to execute - interval_ms: Option, // If Some(), this is a repeating timer -} - -impl PartialEq for Timer { - fn eq(&self, other: &Self) -> bool { - self.fire_time == other.fire_time - } -} - -impl Eq for Timer {} - -impl PartialOrd for Timer { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for Timer { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - // Reverse order for min-heap behavior - other.fire_time.cmp(&self.fire_time) - } -} - -/// Global timer queue -#[derive(Debug)] -struct TimerQueue { - timers: BinaryHeap, - next_id: u32, -} - -impl TimerQueue { - fn new() -> Self { - Self { - timers: BinaryHeap::new(), - next_id: 1, - } - } - - fn add_timer( - &mut self, - delay_ms: u32, - repeat: bool, - callback: String, - reuse_id: Option, - ) -> u32 { - let now = Self::now(); - - let id = reuse_id.unwrap_or_else(|| { - let id = self.next_id; - self.next_id += 1; - id - }); - - let timer = Timer { - id, - fire_time: now + delay_ms as u64, - callback, - interval_ms: if repeat { Some(delay_ms) } else { None }, - }; - - self.timers.push(timer); - id - } - - fn remove_timer(&mut self, timer_id: u32) -> bool { - let original_len = self.timers.len(); - self.timers.retain(|timer| timer.id != timer_id); - self.timers.len() != original_len - } - - fn get_expired_timers(&mut self) -> Vec { - let now = Self::now(); - let mut expired = Vec::new(); - while let Some(timer) = self.timers.peek() { - if timer.fire_time <= now { - expired.push(self.timers.pop().unwrap()); - } else { - break; - } - } - - expired - } - - fn has_pending_timers(&self) -> bool { - !self.timers.is_empty() - } - - fn now() -> u64 { - SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_millis() as u64 - } -} - fn set_timeout<'js>(queue: &Arc>, args: Args<'js>) -> Result> { let (ctx, args) = args.release(); let args = args.into_inner(); @@ -317,28 +213,6 @@ mod tests { use crate::{Config, Runtime}; use anyhow::Error; - #[test] - fn test_timer_queue() { - let mut queue = TimerQueue::new(); - - // Add some timers - let id1 = queue.add_timer(100, false, "console.log('timer1')".to_string(), None); - let id2 = queue.add_timer(50, false, "console.log('timer2')".to_string(), None); - let id3 = queue.add_timer(200, false, "console.log('timer3')".to_string(), None); - - assert_eq!(id1, 1); - assert_eq!(id2, 2); - assert_eq!(id3, 3); - - assert!(queue.has_pending_timers()); - - // Remove a timer - assert!(queue.remove_timer(id2)); - assert!(!queue.remove_timer(999)); // Non-existent timer - - assert!(queue.has_pending_timers()); - } - #[test] fn test_register() -> Result<()> { let mut config = Config::default(); diff --git a/crates/javy/src/apis/timers/queue.rs b/crates/javy/src/apis/timers/queue.rs new file mode 100644 index 00000000..e85f5ab0 --- /dev/null +++ b/crates/javy/src/apis/timers/queue.rs @@ -0,0 +1,134 @@ +use std::{ + collections::BinaryHeap, + time::{SystemTime, UNIX_EPOCH}, +}; + +/// Timer entry in the timer queue +#[derive(Debug, Clone)] +pub(super) struct Timer { + pub id: u32, + pub fire_time: u64, // milliseconds since UNIX epoch + pub callback: String, // JavaScript code to execute + pub interval_ms: Option, // If Some(), this is a repeating timer +} + +impl PartialEq for Timer { + fn eq(&self, other: &Self) -> bool { + self.fire_time == other.fire_time + } +} + +impl Eq for Timer {} + +impl PartialOrd for Timer { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Timer { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + // Reverse order for min-heap behavior + other.fire_time.cmp(&self.fire_time) + } +} + +/// Global timer queue +#[derive(Debug)] +pub(super) struct TimerQueue { + timers: BinaryHeap, + next_id: u32, +} + +impl TimerQueue { + pub fn new() -> Self { + Self { + timers: BinaryHeap::new(), + next_id: 1, + } + } + + pub fn add_timer( + &mut self, + delay_ms: u32, + repeat: bool, + callback: String, + reuse_id: Option, + ) -> u32 { + let now = Self::now(); + + let id = reuse_id.unwrap_or_else(|| { + let id = self.next_id; + self.next_id += 1; + id + }); + + let timer = Timer { + id, + fire_time: now + delay_ms as u64, + callback, + interval_ms: if repeat { Some(delay_ms) } else { None }, + }; + + self.timers.push(timer); + id + } + + pub fn remove_timer(&mut self, timer_id: u32) -> bool { + let original_len = self.timers.len(); + self.timers.retain(|timer| timer.id != timer_id); + self.timers.len() != original_len + } + + pub fn get_expired_timers(&mut self) -> Vec { + let now = Self::now(); + let mut expired = Vec::new(); + while let Some(timer) = self.timers.peek() { + if timer.fire_time <= now { + expired.push(self.timers.pop().unwrap()); + } else { + break; + } + } + + expired + } + + pub fn has_pending_timers(&self) -> bool { + !self.timers.is_empty() + } + + fn now() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_millis() as u64 + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_timer_queue() { + let mut queue = TimerQueue::new(); + + // Add some timers + let id1 = queue.add_timer(100, false, "console.log('timer1')".to_string(), None); + let id2 = queue.add_timer(50, false, "console.log('timer2')".to_string(), None); + let id3 = queue.add_timer(200, false, "console.log('timer3')".to_string(), None); + + assert_eq!(id1, 1); + assert_eq!(id2, 2); + assert_eq!(id3, 3); + + assert!(queue.has_pending_timers()); + + // Remove a timer + assert!(queue.remove_timer(id2)); + assert!(!queue.remove_timer(999)); // Non-existent timer + + assert!(queue.has_pending_timers()); + } +} From 000b2e6b5a7d47868df0f898fd73871f46ed9410 Mon Sep 17 00:00:00 2001 From: Ilya Denisov Date: Sat, 24 May 2025 03:03:45 +0300 Subject: [PATCH 4/8] Refine tests a little --- crates/javy/src/apis/timers/mod.rs | 273 +++++++++++------------------ 1 file changed, 100 insertions(+), 173 deletions(-) diff --git a/crates/javy/src/apis/timers/mod.rs b/crates/javy/src/apis/timers/mod.rs index aa158cd3..b060bde9 100644 --- a/crates/javy/src/apis/timers/mod.rs +++ b/crates/javy/src/apis/timers/mod.rs @@ -26,56 +26,32 @@ impl TimersRuntime { let globals = this.globals(); let queue = self.queue.clone(); - globals.set( - "setTimeout", - Function::new( - this.clone(), - MutFn::new(move |cx, args| { - let (cx, args) = hold_and_release!(cx, args); - set_timeout(&queue, hold!(cx.clone(), args)) - .map_err(|e| to_js_error(cx, e)) - }), - )?, - )?; + globals.set("setTimeout", Function::new(this.clone(), MutFn::new(move |cx, args| { + let (cx, args) = hold_and_release!(cx, args); + set_timeout(&queue, hold!(cx.clone(), args)) + .map_err(|e| to_js_error(cx, e)) + }))?)?; let queue = self.queue.clone(); - globals.set( - "clearTimeout", - Function::new( - this.clone(), - MutFn::new(move |cx, args| { - let (cx, args) = hold_and_release!(cx, args); - clear_timeout(&queue, hold!(cx.clone(), args)) - .map_err(|e| to_js_error(cx, e)) - }), - )?, - )?; + globals.set("clearTimeout",Function::new(this.clone(), MutFn::new(move |cx, args| { + let (cx, args) = hold_and_release!(cx, args); + clear_timeout(&queue, hold!(cx.clone(), args)) + .map_err(|e| to_js_error(cx, e)) + }))?)?; let queue = self.queue.clone(); - globals.set( - "setInterval", - Function::new( - this.clone(), - MutFn::new(move |cx, args| { - let (cx, args) = hold_and_release!(cx, args); - set_interval(&queue, hold!(cx.clone(), args)) - .map_err(|e| to_js_error(cx, e)) - }), - )?, - )?; + globals.set("setInterval", Function::new(this.clone(), MutFn::new(move |cx, args| { + let (cx, args) = hold_and_release!(cx, args); + set_interval(&queue, hold!(cx.clone(), args)) + .map_err(|e| to_js_error(cx, e)) + }))?)?; let queue = self.queue.clone(); - globals.set( - "clearInterval", - Function::new( - this.clone(), - MutFn::new(move |cx, args| { - let (cx, args) = hold_and_release!(cx, args); - clear_interval(&queue, hold!(cx.clone(), args)) - .map_err(|e| to_js_error(cx, e)) - }), - )?, - )?; + globals.set("clearInterval", Function::new(this.clone(), MutFn::new(move |cx, args| { + let (cx, args) = hold_and_release!(cx, args); + clear_interval(&queue, hold!(cx.clone(), args)) + .map_err(|e| to_js_error(cx, e)) + }))?)?; Ok(()) } @@ -220,13 +196,19 @@ mod tests { let runtime = Runtime::new(config)?; runtime.context().with(|cx| { // Check that setTimeout is available - let result: Value = cx.eval("typeof setTimeout")?; - let type_str = val_to_string(&cx, result)?; + let type_str: String = cx.eval("typeof setTimeout")?; assert_eq!(type_str, "function"); // Check that clearTimeout is available - let result: Value = cx.eval("typeof clearTimeout")?; - let type_str = val_to_string(&cx, result)?; + let type_str: String = cx.eval("typeof clearTimeout")?; + assert_eq!(type_str, "function"); + + // Check that setInterval is available + let type_str: String = cx.eval("typeof setInterval")?; + assert_eq!(type_str, "function"); + + // Check that clearInterval is available + let type_str: String = cx.eval("typeof clearInterval")?; assert_eq!(type_str, "function"); Ok::<_, Error>(()) @@ -241,13 +223,11 @@ mod tests { let runtime = Runtime::new(config)?; runtime.context().with(|cx| { // Test setTimeout with string callback - let result: Value = cx.eval("setTimeout('1+1', 100)")?; - let timer_id = result.as_number().unwrap() as i32; + let timer_id: i32 = cx.eval("setTimeout('1+1', 100)")?; assert!(timer_id > 0); // Test setTimeout with function callback - let result: Value = cx.eval("setTimeout(function() { return 42; }, 50)")?; - let timer_id2 = result.as_number().unwrap() as i32; + let timer_id2: i32 = cx.eval("setTimeout(function() { return 42; }, 50)")?; assert!(timer_id2 > timer_id); Ok::<_, Error>(()) @@ -262,12 +242,9 @@ mod tests { let runtime = Runtime::new(config)?; runtime.context().with(|cx| { // Create a timer and clear it - let result: Value = cx.eval( - "const id = setTimeout('console.log(\"test\")', 1000); clearTimeout(id); id", - )?; - let timer_id = result.as_number().unwrap() as i32; + let code = "const id = setTimeout('console.log(\"test\")', 1000); clearTimeout(id); id"; + let timer_id: i32 = cx.eval(code)?; assert!(timer_id > 0); - Ok::<_, Error>(()) })?; Ok(()) @@ -283,11 +260,10 @@ mod tests { let unique_var = format!("timerExecuted_{}", std::process::id()); runtime.context().with(|cx| { - let timer_code = format!( + cx.eval::<(), _>(format!( "globalThis.{} = false; setTimeout('globalThis.{} = true', 0)", unique_var, unique_var - ); - cx.eval::<(), _>(timer_code.as_str())?; + ))?; Ok::<_, Error>(()) })?; @@ -296,14 +272,10 @@ mod tests { runtime.context().with(|cx| { // Check if timer was executed - let check_code = format!("globalThis.{}", unique_var); - let result: Value = cx.eval(check_code.as_str())?; - - assert!(result.as_bool().unwrap_or(false)); - + let result: bool = cx.eval(format!("globalThis.{}", unique_var))?; + assert!(result); Ok::<_, Error>(()) })?; - Ok(()) } @@ -318,11 +290,10 @@ mod tests { runtime.context().with(|cx| { // Set a timer with a delay that shouldn't fire immediately - let timer_code = format!( + cx.eval::<(), _>(format!( "globalThis.{} = false; setTimeout('globalThis.{} = true', 1000)", unique_var, unique_var - ); - cx.eval::<(), _>(timer_code.as_str())?; + ))?; Ok::<_, Error>(()) })?; @@ -331,10 +302,8 @@ mod tests { runtime.context().with(|cx| { // Check if timer was NOT executed - let check_code = format!("globalThis.{}", unique_var); - let result: Value = cx.eval(check_code.as_str())?; - assert!(!result.as_bool().unwrap_or(true)); - + let result: bool = cx.eval(format!("globalThis.{}", unique_var))?; + assert_eq!(result, false); Ok::<_, Error>(()) })?; Ok(()) @@ -353,16 +322,15 @@ mod tests { runtime.context().with(|cx| { // Set multiple timers - let timer_code = format!( + cx.eval::<(), _>(format!( " - globalThis.{} = false; - globalThis.{} = false; - setTimeout('globalThis.{} = true', 0); - setTimeout('globalThis.{} = true', 0); - ", + globalThis.{} = false; + globalThis.{} = false; + setTimeout('globalThis.{} = true', 0); + setTimeout('globalThis.{} = true', 0); + ", timer1_var, timer2_var, timer1_var, timer2_var - ); - cx.eval::<(), _>(timer_code.as_str())?; + ))?; Ok::<_, Error>(()) })?; @@ -371,12 +339,10 @@ mod tests { runtime.context().with(|cx| { // Check if both timers were executed - let check1_code = format!("globalThis.{}", timer1_var); - let check2_code = format!("globalThis.{}", timer2_var); - let result1: Value = cx.eval(check1_code.as_str())?; - let result2: Value = cx.eval(check2_code.as_str())?; - assert!(result1.as_bool().unwrap_or(false)); - assert!(result2.as_bool().unwrap_or(false)); + let result1: bool = cx.eval(format!("globalThis.{}", timer1_var))?; + let result2: bool = cx.eval(format!("globalThis.{}", timer2_var))?; + assert_eq!(result1, true); + assert_eq!(result2, true); Ok::<_, Error>(()) })?; @@ -394,15 +360,14 @@ mod tests { runtime.context().with(|cx| { // Set a timer and immediately clear it - let timer_code = format!( + cx.eval::<(), _>(format!( " - globalThis.{} = false; - const id = setTimeout('globalThis.{} = true', 0); - clearTimeout(id); - ", + globalThis.{} = false; + const id = setTimeout('globalThis.{} = true', 0); + clearTimeout(id); + ", unique_var, unique_var - ); - cx.eval::<(), _>(timer_code.as_str())?; + ))?; Ok::<_, Error>(()) })?; @@ -411,10 +376,8 @@ mod tests { runtime.context().with(|cx| { // Check if timer was NOT executed - let check_code = format!("globalThis.{}", unique_var); - let result: Value = cx.eval(check_code.as_str())?; - assert!(!result.as_bool().unwrap_or(true)); - + let result: bool = cx.eval(format!("globalThis.{}", unique_var))?; + assert_eq!(result, false); Ok::<_, Error>(()) })?; Ok(()) @@ -432,12 +395,12 @@ mod tests { runtime.context().with(|cx| { // Add a timer cx.eval::<(), _>("setTimeout('console.log(\"test\")', 1000)")?; - - // Should have pending timers - assert!(runtime.has_pending_timers()); - Ok::<_, Error>(()) })?; + + // Should have pending timers + assert!(runtime.has_pending_timers()); + Ok(()) } @@ -448,15 +411,14 @@ mod tests { let runtime = Runtime::new(config)?; runtime.context().with(|cx| { // Test setInterval with string callback - let result: Value = cx.eval("setInterval('1+1', 100)")?; - let interval_id = result.as_number().unwrap() as i32; + let interval_id: i32 = cx.eval("setInterval('1+1', 100)")?; assert!(interval_id > 0); - - // Should have pending timers - assert!(runtime.has_pending_timers()); - Ok::<_, Error>(()) })?; + + // Should have pending timers + assert!(runtime.has_pending_timers()); + Ok(()) } @@ -467,12 +429,9 @@ mod tests { let runtime = Runtime::new(config)?; runtime.context().with(|cx| { // Create an interval and clear it - let result: Value = cx.eval( - "const id = setInterval('console.log(\"test\")', 1000); clearInterval(id); id", - )?; - let interval_id = result.as_number().unwrap() as i32; + let code = "const id = setInterval('console.log(\"test\")', 1000); clearInterval(id); id"; + let interval_id: i32 = cx.eval(code)?; assert!(interval_id > 0); - Ok::<_, Error>(()) })?; Ok(()) @@ -488,11 +447,10 @@ mod tests { let unique_var = format!("intervalCount_{}", std::process::id()); runtime.context().with(|cx| { - let interval_code = format!( + cx.eval::<(), _>(format!( "globalThis.{} = 0; setInterval('globalThis.{}++', 0)", unique_var, unique_var - ); - cx.eval::<(), _>(interval_code.as_str())?; + ))?; Ok::<_, Error>(()) })?; @@ -502,18 +460,9 @@ mod tests { runtime.resolve_pending_jobs()?; runtime.context().with(|cx| { - // Check if interval executed multiple times - let check_code = format!("globalThis.{}", unique_var); - let result: Value = cx.eval(check_code.as_str())?; - let count = result.as_number().unwrap_or(0.0) as i32; - - // Should have executed at least twice (showing it's repeating) - assert!( - count >= 2, - "Interval should have executed multiple times, got {}", - count - ); - + // Check if interval executed multiple times (showing it's repeating) + let count: i32 = cx.eval(format!("globalThis.{}", unique_var))?; + assert!(count >= 2, "Interval should have executed multiple times, got {}", count); Ok::<_, Error>(()) })?; Ok(()) @@ -529,15 +478,14 @@ mod tests { let unique_var = format!("clearedIntervalCount_{}", std::process::id()); runtime.context().with(|cx| { - let interval_code = format!( + cx.eval::<(), _>(format!( " - globalThis.{} = 0; - const id = setInterval('globalThis.{}++', 0); - clearInterval(id); - ", + globalThis.{} = 0; + const id = setInterval('globalThis.{}++', 0); + clearInterval(id); + ", unique_var, unique_var - ); - cx.eval::<(), _>(interval_code.as_str())?; + ))?; Ok::<_, Error>(()) })?; @@ -547,18 +495,9 @@ mod tests { runtime.resolve_pending_jobs()?; runtime.context().with(|cx| { - // Check that interval was NOT executed - let check_code = format!("globalThis.{}", unique_var); - let result: Value = cx.eval(check_code.as_str())?; - let count = result.as_number().unwrap_or(-1.0) as i32; - - // Should still be 0 (not executed) - assert_eq!( - count, 0, - "Cleared interval should not execute, got {}", - count - ); - + // Check that interval was NOT executed (should be 0) + let count: i32 = cx.eval(format!("globalThis.{}", unique_var))?; + assert_eq!(count, 0, "Cleared interval should not execute"); Ok::<_, Error>(()) })?; Ok(()) @@ -579,14 +518,14 @@ mod tests { // Set timeout first, then interval - both with 0 delay let timer_code = format!( " - globalThis.{} = false; - globalThis.{} = 0; - setTimeout('globalThis.{} = true', 0); - setInterval('globalThis.{}++', 0); - ", + globalThis.{} = false; + globalThis.{} = 0; + setTimeout('globalThis.{} = true', 0); + setInterval('globalThis.{}++', 0); + ", timeout_var, interval_var, timeout_var, interval_var ); - cx.eval::<(), _>(timer_code.as_str())?; + cx.eval::<(), _>(timer_code)?; Ok::<_, Error>(()) })?; @@ -598,17 +537,11 @@ mod tests { let interval_check = format!("globalThis.{}", interval_var); runtime.context().with(|cx| { - let timeout_result: Value = cx.eval(timeout_check.as_str())?; - let interval_result: Value = cx.eval(interval_check.as_str())?; + let timeout_result: bool = cx.eval(timeout_check.as_str())?; + let interval_result: i32 = cx.eval(interval_check.as_str())?; - assert!( - timeout_result.as_bool().unwrap_or(false), - "Timeout should have executed" - ); - assert!( - interval_result.as_number().unwrap_or(0.0) as i32 >= 1, - "Interval should have executed at least once" - ); + assert!(timeout_result, "Timeout should have executed"); + assert!(interval_result >= 1, "Interval should have executed at least once"); Ok::<_, Error>(()) })?; @@ -617,18 +550,12 @@ mod tests { runtime.resolve_pending_jobs()?; runtime.context().with(|cx| { - let interval_result2: Value = cx.eval(interval_check.as_str())?; - let timeout_result2: Value = cx.eval(timeout_check.as_str())?; + let timeout_result: bool = cx.eval(timeout_check.as_str())?; + let interval_result: i32 = cx.eval(interval_check.as_str())?; // Timeout should still be true (unchanged), interval should have incremented - assert!( - timeout_result2.as_bool().unwrap_or(false), - "Timeout should remain executed" - ); - assert!( - interval_result2.as_number().unwrap_or(0.0) as i32 >= 2, - "Interval should have executed multiple times" - ); + assert!(timeout_result, "Timeout should remain executed"); + assert!(interval_result >= 2, "Interval should have executed multiple times"); Ok::<_, Error>(()) })?; From 9dbe7c0c5aa1aa230b68f309715bb359d59a8d9b Mon Sep 17 00:00:00 2001 From: Ilya Denisov Date: Sat, 24 May 2025 14:01:01 +0300 Subject: [PATCH 5/8] Remove pointless complexity from tests --- crates/javy/src/apis/timers/mod.rs | 168 +++++++++-------------------- 1 file changed, 50 insertions(+), 118 deletions(-) diff --git a/crates/javy/src/apis/timers/mod.rs b/crates/javy/src/apis/timers/mod.rs index b060bde9..9f9f3a56 100644 --- a/crates/javy/src/apis/timers/mod.rs +++ b/crates/javy/src/apis/timers/mod.rs @@ -195,22 +195,11 @@ mod tests { config.timers(true); let runtime = Runtime::new(config)?; runtime.context().with(|cx| { - // Check that setTimeout is available - let type_str: String = cx.eval("typeof setTimeout")?; - assert_eq!(type_str, "function"); - - // Check that clearTimeout is available - let type_str: String = cx.eval("typeof clearTimeout")?; - assert_eq!(type_str, "function"); - - // Check that setInterval is available - let type_str: String = cx.eval("typeof setInterval")?; - assert_eq!(type_str, "function"); - - // Check that clearInterval is available - let type_str: String = cx.eval("typeof clearInterval")?; - assert_eq!(type_str, "function"); - + // Check that API is available + assert_eq!("function", cx.eval::("typeof setTimeout")?); + assert_eq!("function", cx.eval::("typeof clearTimeout")?); + assert_eq!("function", cx.eval::("typeof setInterval")?); + assert_eq!("function", cx.eval::("typeof clearInterval")?); Ok::<_, Error>(()) })?; Ok(()) @@ -242,7 +231,7 @@ mod tests { let runtime = Runtime::new(config)?; runtime.context().with(|cx| { // Create a timer and clear it - let code = "const id = setTimeout('console.log(\"test\")', 1000); clearTimeout(id); id"; + let code = r#"const id = setTimeout('console.log("test")', 1000); clearTimeout(id); id"#; let timer_id: i32 = cx.eval(code)?; assert!(timer_id > 0); Ok::<_, Error>(()) @@ -256,14 +245,8 @@ mod tests { config.timers(true); let runtime = Runtime::new(config)?; - // Use a unique variable name to avoid interference between tests - let unique_var = format!("timerExecuted_{}", std::process::id()); - runtime.context().with(|cx| { - cx.eval::<(), _>(format!( - "globalThis.{} = false; setTimeout('globalThis.{} = true', 0)", - unique_var, unique_var - ))?; + cx.eval::<(), _>("globalThis.var1 = -123; setTimeout('globalThis.var1 = 321', 0)")?; Ok::<_, Error>(()) })?; @@ -272,8 +255,7 @@ mod tests { runtime.context().with(|cx| { // Check if timer was executed - let result: bool = cx.eval(format!("globalThis.{}", unique_var))?; - assert!(result); + assert_eq!(321, cx.eval::("globalThis.var1")?); Ok::<_, Error>(()) })?; Ok(()) @@ -285,15 +267,9 @@ mod tests { config.timers(true); let runtime = Runtime::new(config)?; - // Use unique variable name to avoid interference between tests - let unique_var = format!("delayedTimer_{}", std::process::id()); - runtime.context().with(|cx| { // Set a timer with a delay that shouldn't fire immediately - cx.eval::<(), _>(format!( - "globalThis.{} = false; setTimeout('globalThis.{} = true', 1000)", - unique_var, unique_var - ))?; + cx.eval::<(), _>("globalThis.var1 = -765; setTimeout('globalThis.var1 = 567', 1000)")?; Ok::<_, Error>(()) })?; @@ -302,8 +278,7 @@ mod tests { runtime.context().with(|cx| { // Check if timer was NOT executed - let result: bool = cx.eval(format!("globalThis.{}", unique_var))?; - assert_eq!(result, false); + assert_eq!(-765, cx.eval::("globalThis.var1")?); Ok::<_, Error>(()) })?; Ok(()) @@ -315,22 +290,14 @@ mod tests { config.timers(true); let runtime = Runtime::new(config)?; - // Use unique variable names to avoid interference between tests - let unique_id = std::process::id(); - let timer1_var = format!("timer1_{}", unique_id); - let timer2_var = format!("timer2_{}", unique_id); - runtime.context().with(|cx| { // Set multiple timers - cx.eval::<(), _>(format!( - " - globalThis.{} = false; - globalThis.{} = false; - setTimeout('globalThis.{} = true', 0); - setTimeout('globalThis.{} = true', 0); - ", - timer1_var, timer2_var, timer1_var, timer2_var - ))?; + cx.eval::<(), _>(" + globalThis.var1 = 0; + globalThis.var2 = 0; + setTimeout('globalThis.var1 = 123', 0); + setTimeout('globalThis.var2 = 321', 0); + ")?; Ok::<_, Error>(()) })?; @@ -339,11 +306,8 @@ mod tests { runtime.context().with(|cx| { // Check if both timers were executed - let result1: bool = cx.eval(format!("globalThis.{}", timer1_var))?; - let result2: bool = cx.eval(format!("globalThis.{}", timer2_var))?; - assert_eq!(result1, true); - assert_eq!(result2, true); - + assert_eq!(123, cx.eval::("globalThis.var1")?); + assert_eq!(321, cx.eval::("globalThis.var2")?); Ok::<_, Error>(()) })?; Ok(()) @@ -355,19 +319,13 @@ mod tests { config.timers(true); let runtime = Runtime::new(config)?; - // Use unique variable name to avoid interference between tests - let unique_var = format!("clearedTimer_{}", std::process::id()); - runtime.context().with(|cx| { // Set a timer and immediately clear it - cx.eval::<(), _>(format!( - " - globalThis.{} = false; - const id = setTimeout('globalThis.{} = true', 0); - clearTimeout(id); - ", - unique_var, unique_var - ))?; + cx.eval::<(), _>(" + globalThis.var1 = -432; + const id = setTimeout('globalThis.var1 = 234', 0); + clearTimeout(id); + ")?; Ok::<_, Error>(()) })?; @@ -376,8 +334,7 @@ mod tests { runtime.context().with(|cx| { // Check if timer was NOT executed - let result: bool = cx.eval(format!("globalThis.{}", unique_var))?; - assert_eq!(result, false); + assert_eq!(-432, cx.eval::("globalThis.var1")?); Ok::<_, Error>(()) })?; Ok(()) @@ -394,7 +351,7 @@ mod tests { runtime.context().with(|cx| { // Add a timer - cx.eval::<(), _>("setTimeout('console.log(\"test\")', 1000)")?; + cx.eval::<(), _>(r#"setTimeout('console.log("test")', 1000)"#)?; Ok::<_, Error>(()) })?; @@ -429,7 +386,7 @@ mod tests { let runtime = Runtime::new(config)?; runtime.context().with(|cx| { // Create an interval and clear it - let code = "const id = setInterval('console.log(\"test\")', 1000); clearInterval(id); id"; + let code = r#"const id = setInterval('console.log("test")', 1000); clearInterval(id); id"#; let interval_id: i32 = cx.eval(code)?; assert!(interval_id > 0); Ok::<_, Error>(()) @@ -443,14 +400,8 @@ mod tests { config.timers(true); let runtime = Runtime::new(config)?; - // Use unique variable name to avoid interference between tests - let unique_var = format!("intervalCount_{}", std::process::id()); - runtime.context().with(|cx| { - cx.eval::<(), _>(format!( - "globalThis.{} = 0; setInterval('globalThis.{}++', 0)", - unique_var, unique_var - ))?; + cx.eval::<(), _>("globalThis.var1 = 1000; setInterval('globalThis.var1++', 0)")?; Ok::<_, Error>(()) })?; @@ -461,8 +412,8 @@ mod tests { runtime.context().with(|cx| { // Check if interval executed multiple times (showing it's repeating) - let count: i32 = cx.eval(format!("globalThis.{}", unique_var))?; - assert!(count >= 2, "Interval should have executed multiple times, got {}", count); + let var1: i32 = cx.eval("globalThis.var1")?; + assert!(var1 >= 1002, "Interval should have executed multiple times, got {}", var1); Ok::<_, Error>(()) })?; Ok(()) @@ -474,18 +425,12 @@ mod tests { config.timers(true); let runtime = Runtime::new(config)?; - // Use unique variable name to avoid interference between tests - let unique_var = format!("clearedIntervalCount_{}", std::process::id()); - runtime.context().with(|cx| { - cx.eval::<(), _>(format!( - " - globalThis.{} = 0; - const id = setInterval('globalThis.{}++', 0); - clearInterval(id); - ", - unique_var, unique_var - ))?; + cx.eval::<(), _>(" + globalThis.var1 = 100; + const id = setInterval('globalThis.var1++', 0); + clearInterval(id); + ")?; Ok::<_, Error>(()) })?; @@ -496,8 +441,8 @@ mod tests { runtime.context().with(|cx| { // Check that interval was NOT executed (should be 0) - let count: i32 = cx.eval(format!("globalThis.{}", unique_var))?; - assert_eq!(count, 0, "Cleared interval should not execute"); + let var1: i32 = cx.eval("globalThis.var1")?; + assert_eq!(100, var1, "Cleared interval should not execute"); Ok::<_, Error>(()) })?; Ok(()) @@ -509,39 +454,26 @@ mod tests { config.timers(true); let runtime = Runtime::new(config)?; - // Use unique variable names - let unique_id = std::process::id(); - let timeout_var = format!("timeoutExecuted_{}", unique_id); - let interval_var = format!("intervalCount_{}", unique_id); - runtime.context().with(|cx| { // Set timeout first, then interval - both with 0 delay - let timer_code = format!( - " - globalThis.{} = false; - globalThis.{} = 0; - setTimeout('globalThis.{} = true', 0); - setInterval('globalThis.{}++', 0); - ", - timeout_var, interval_var, timeout_var, interval_var - ); - cx.eval::<(), _>(timer_code)?; + cx.eval::<(), _>(" + globalThis.var1 = -543; + globalThis.var2 = 100; + setTimeout('globalThis.var1 = 999', 0); + setInterval('globalThis.var2++', 0); + ")?; Ok::<_, Error>(()) })?; // Process timers first time - should execute both timeout and first interval runtime.resolve_pending_jobs()?; - // Check both timeout and interval results - let timeout_check = format!("globalThis.{}", timeout_var); - let interval_check = format!("globalThis.{}", interval_var); - runtime.context().with(|cx| { - let timeout_result: bool = cx.eval(timeout_check.as_str())?; - let interval_result: i32 = cx.eval(interval_check.as_str())?; + let var1: i32 = cx.eval("globalThis.var1")?; + let var2: i32 = cx.eval("globalThis.var2")?; - assert!(timeout_result, "Timeout should have executed"); - assert!(interval_result >= 1, "Interval should have executed at least once"); + assert_eq!(999, var1, "Timeout should have executed"); + assert!(var2 >= 101, "Interval should have executed at least once, got {}", var2); Ok::<_, Error>(()) })?; @@ -550,12 +482,12 @@ mod tests { runtime.resolve_pending_jobs()?; runtime.context().with(|cx| { - let timeout_result: bool = cx.eval(timeout_check.as_str())?; - let interval_result: i32 = cx.eval(interval_check.as_str())?; + let var1: i32 = cx.eval("globalThis.var1")?; + let var2: i32 = cx.eval("globalThis.var2")?; // Timeout should still be true (unchanged), interval should have incremented - assert!(timeout_result, "Timeout should remain executed"); - assert!(interval_result >= 2, "Interval should have executed multiple times"); + assert_eq!(999, var1, "Timeout should remain executed"); + assert!(var2 >= 102, "Interval should have executed multiple times"); Ok::<_, Error>(()) })?; From 0e3c65991e59f4203d76f218180b6afb019d02ff Mon Sep 17 00:00:00 2001 From: Ilya Denisov Date: Sat, 24 May 2025 15:52:47 +0300 Subject: [PATCH 6/8] WIP add test to cover function callbacks (as opposed to string ones) The test naturally fails, as the current implementation actually only handles string callbacks that are eval-ed, but preserving some reference to a closure (with proper context binding) does not seems to be trivial (due to lack of the expertise on the project on my side). --- crates/javy/src/apis/timers/mod.rs | 38 ++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/crates/javy/src/apis/timers/mod.rs b/crates/javy/src/apis/timers/mod.rs index 9f9f3a56..5341ec7d 100644 --- a/crates/javy/src/apis/timers/mod.rs +++ b/crates/javy/src/apis/timers/mod.rs @@ -261,6 +261,44 @@ mod tests { Ok(()) } + #[test] + fn test_timer_function_callback() -> Result<()> { + let mut config = Config::default(); + config.timers(true); + let runtime = Runtime::new(config)?; + + runtime.context().with(|cx| { + // Create timeout with a closure with a mutable state + // To make sure the closure preserves the state reference + cx.eval::<(), _>(" + globalThis.var1 = -123; + function createIncrementor(initialDelta) { + let delta = initialDelta; + return [function() { globalThis.var1 += delta; }, function(newDelta) { delta = newDelta; }]; + } + [incrementor, setDelta] = createIncrementor(100); + incrementor(); + setTimeout(incrementor, 0); + setDelta(123); + ")?; + + // So far, only explicit call to incrementor (having delta = 100) is done + assert_eq!(-23, cx.eval::("globalThis.var1")?); + + Ok::<_, Error>(()) + })?; + + // Process timers immediately without sleep - they should be available + runtime.resolve_pending_jobs()?; + + runtime.context().with(|cx| { + // Check if closure correctly applied delta modified after its creation + assert_eq!(100, cx.eval::("globalThis.var1")?); + Ok::<_, Error>(()) + })?; + Ok(()) + } + #[test] fn test_timer_with_delay() -> Result<()> { let mut config = Config::default(); From 441ae7fa927c0f42547014dbaf367fa39bf46eea Mon Sep 17 00:00:00 2001 From: Ilya Denisov Date: Sat, 24 May 2025 18:15:42 +0300 Subject: [PATCH 7/8] HACK function callback support --- crates/javy/src/apis/timers/mod.rs | 80 +++++++++++++++++++--------- crates/javy/src/apis/timers/queue.rs | 22 +++++--- 2 files changed, 70 insertions(+), 32 deletions(-) diff --git a/crates/javy/src/apis/timers/mod.rs b/crates/javy/src/apis/timers/mod.rs index 5341ec7d..22b90318 100644 --- a/crates/javy/src/apis/timers/mod.rs +++ b/crates/javy/src/apis/timers/mod.rs @@ -1,7 +1,7 @@ use std::sync::{Arc, Mutex}; mod queue; -use queue::TimerQueue; +use queue::{TimerCallback, TimerQueue}; use crate::{ hold, hold_and_release, @@ -72,9 +72,23 @@ impl TimersRuntime { // Execute all timer callbacks (both timeouts and intervals) for timer in &expired_timers { - if let Err(e) = ctx.eval::<(), _>(timer.callback.as_str()) { - eprintln!("Timer callback error: {}", e); - } + match &timer.callback { + TimerCallback::Code(code) => { + if let Err(e) = ctx.eval::<(), _>(code.as_str()) { + eprintln!("Timer callback error: {}", e); + } + }, + TimerCallback::Function => { + let code = format!("globalThis.__timer_callback_{}()", timer.id); + if let Err(e) = ctx.eval::<(), _>(code.as_str()) { + eprintln!("Timer callback error: {}", e); + } + // remove the callback from the global object, unless it's an interval + if timer.interval_ms.is_none() { + ctx.globals().remove(format!("__timer_callback_{}", timer.id))?; + } + }, + }; } Ok(()) @@ -95,13 +109,12 @@ fn set_timeout<'js>(queue: &Arc>, args: Args<'js>) -> Result(queue: &Arc>, args: Args<'js>) -> Result(queue: &Arc>, args: Args<'js>) -> Result let timer_id = args[0].as_number().unwrap_or(0.0) as u32; let mut queue = queue.lock().unwrap(); - queue.remove_timer(timer_id); + let removed = queue.remove_timer(timer_id); drop(queue); + if removed { + ctx.globals().remove(format!("__timer_callback_{}", timer_id))?; + } + Ok(Value::new_undefined(ctx)) } @@ -143,13 +164,12 @@ fn set_interval<'js>(queue: &Arc>, args: Args<'js>) -> Result< return Err(anyhow!("setInterval requires at least 1 argument")); } - // Get callback (can be function or string) - let callback_str = if args[0].is_function() { - // Convert function to string representation - val_to_string(&ctx, args[0].clone())? - } else { - // Treat as string code - val_to_string(&ctx, args[0].clone())? + let callback_str = val_to_string(&ctx, args[0].clone())?; + let callback = if args[0].is_function() { + TimerCallback::Function + } + else { + TimerCallback::Code(callback_str) }; // Get interval (default to 0 if not provided) @@ -160,9 +180,13 @@ fn set_interval<'js>(queue: &Arc>, args: Args<'js>) -> Result< }; let mut queue = queue.lock().unwrap(); - let timer_id = queue.add_timer(interval_ms, true, callback_str, None); + let timer_id = queue.add_timer(interval_ms, true, callback, None); drop(queue); + if args[0].is_function() { + ctx.globals().set(format!("__timer_callback_{}", timer_id), args[0].clone())?; + } + Ok(Value::new_int(ctx, timer_id as i32)) } @@ -177,9 +201,13 @@ fn clear_interval<'js>(queue: &Arc>, args: Args<'js>) -> Resul let timer_id = args[0].as_number().unwrap_or(0.0) as u32; let mut queue = queue.lock().unwrap(); - queue.remove_timer(timer_id); + let removed = queue.remove_timer(timer_id); drop(queue); + if removed { + ctx.globals().remove(format!("__timer_callback_{}", timer_id))?; + } + Ok(Value::new_undefined(ctx)) } @@ -270,13 +298,13 @@ mod tests { runtime.context().with(|cx| { // Create timeout with a closure with a mutable state // To make sure the closure preserves the state reference - cx.eval::<(), _>(" + let res = cx.eval::<(), _>(" globalThis.var1 = -123; function createIncrementor(initialDelta) { - let delta = initialDelta; - return [function() { globalThis.var1 += delta; }, function(newDelta) { delta = newDelta; }]; + var delta = initialDelta; + return [() => globalThis.var1 += delta, (newDelta) => delta = newDelta]; } - [incrementor, setDelta] = createIncrementor(100); + var [incrementor, setDelta] = createIncrementor(100); incrementor(); setTimeout(incrementor, 0); setDelta(123); diff --git a/crates/javy/src/apis/timers/queue.rs b/crates/javy/src/apis/timers/queue.rs index e85f5ab0..86cad2a2 100644 --- a/crates/javy/src/apis/timers/queue.rs +++ b/crates/javy/src/apis/timers/queue.rs @@ -3,12 +3,18 @@ use std::{ time::{SystemTime, UNIX_EPOCH}, }; -/// Timer entry in the timer queue #[derive(Debug, Clone)] +pub(super) enum TimerCallback { + Code(String), + Function, +} + +/// Timer entry in the timer queue +#[derive(Debug)] pub(super) struct Timer { pub id: u32, pub fire_time: u64, // milliseconds since UNIX epoch - pub callback: String, // JavaScript code to execute + pub callback: TimerCallback, pub interval_ms: Option, // If Some(), this is a repeating timer } @@ -52,7 +58,7 @@ impl TimerQueue { &mut self, delay_ms: u32, repeat: bool, - callback: String, + callback: TimerCallback, reuse_id: Option, ) -> u32 { let now = Self::now(); @@ -114,10 +120,14 @@ mod tests { fn test_timer_queue() { let mut queue = TimerQueue::new(); + fn add_timer(delay_ms: u32, callback_code: &str, queue: &mut TimerQueue) -> u32 { + queue.add_timer(delay_ms, false, TimerCallback::Code(callback_code.to_string()), None) + } + // Add some timers - let id1 = queue.add_timer(100, false, "console.log('timer1')".to_string(), None); - let id2 = queue.add_timer(50, false, "console.log('timer2')".to_string(), None); - let id3 = queue.add_timer(200, false, "console.log('timer3')".to_string(), None); + let id1 = add_timer(100, "console.log('timer1')", &mut queue); + let id2 = add_timer(50, "console.log('timer2')", &mut queue); + let id3 = add_timer(200, "console.log('timer3')", &mut queue); assert_eq!(id1, 1); assert_eq!(id2, 2); From 1b7eec1e9d5d429984ae8212d509a506d8534eb3 Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Wed, 28 May 2025 10:56:11 +0300 Subject: [PATCH 8/8] Update comment --- crates/javy/src/runtime.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/javy/src/runtime.rs b/crates/javy/src/runtime.rs index 2edd0faa..d26e8e44 100644 --- a/crates/javy/src/runtime.rs +++ b/crates/javy/src/runtime.rs @@ -39,7 +39,7 @@ pub struct Runtime { /// The inner QuickJS runtime representation. // Read above on the usage of `ManuallyDrop`. inner: ManuallyDrop, - /// Whether timers are enabled + /// Timers runtime state, if enabled. timers: Option, }