mirror of
https://github.com/servo/servo.git
synced 2025-08-03 20:50:07 +01:00
Auto merge of #8768 - vegayours:8616_intermittent_option_unwrap_in_timers, r=jdm
fix intermittent Option::unwrap in timers fixes intermittent #8616 This intermittent indicates real problem in code. Lets consider such code: ```javascript // timer 1 setTimeout(function() { //timer 2 setTimeout(function() {}, 0); }, 0); ``` When we receive event to fire timer 1 it will be selected and extracted from active timers list in fire_timer function. During timer 1 handler execution we will schedule timer 2 and request timer event for it. But it will be executed during same fire_timer call because of 0 timeout. And as a result we will have empty timers list and expecting event for timer 2 that will crash in assert. I'm not sure that all I've written is clear, but we have something like this: ``` install timer 1 -> [1] in timers list push and expect timer event 1 -> expected_event=1 received timer event 1 fire_timer() select timer 1 to execute -> [] in timers list execute timer 1 handler install timer 2 -> [2] in timers list push and expect timer event 2 -> expected_event=2 select timer 2 to execute (because of 0 timeout) -> [] in tiemrs list execute timer 2 handler expected_event=2 is dangling received timer event 2 fire_timer() -> BOOM ``` <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8768) <!-- Reviewable:end -->
This commit is contained in:
commit
b32128e299
4 changed files with 55 additions and 8 deletions
|
@ -23,7 +23,7 @@ use std::rc::Rc;
|
|||
use util::mem::HeapSizeOf;
|
||||
use util::str::DOMString;
|
||||
|
||||
#[derive(JSTraceable, PartialEq, Eq, Copy, Clone, HeapSizeOf, Hash, PartialOrd, Ord)]
|
||||
#[derive(JSTraceable, PartialEq, Eq, Copy, Clone, HeapSizeOf, Hash, PartialOrd, Ord, Debug)]
|
||||
pub struct TimerHandle(i32);
|
||||
|
||||
#[derive(JSTraceable, HeapSizeOf)]
|
||||
|
@ -253,16 +253,22 @@ impl ActiveTimers {
|
|||
// Since the event id was the expected one, at least one timer should be due.
|
||||
assert!(base_time >= self.timers.borrow().last().unwrap().next_call);
|
||||
|
||||
// select timers to run to prevent firing timers
|
||||
// that were installed during fire of another timer
|
||||
let mut timers_to_run: Vec<Timer> = Vec::new();
|
||||
|
||||
loop {
|
||||
let timer = {
|
||||
let mut timers = self.timers.borrow_mut();
|
||||
|
||||
if timers.is_empty() || timers.last().unwrap().next_call > base_time {
|
||||
break;
|
||||
}
|
||||
|
||||
timers.pop().unwrap()
|
||||
};
|
||||
timers_to_run.push(timers.pop().unwrap());
|
||||
}
|
||||
|
||||
for timer in timers_to_run {
|
||||
|
||||
let callback = timer.callback.clone();
|
||||
|
||||
// prep for step 6 in nested set_timeout_or_interval calls
|
||||
|
|
|
@ -5757,6 +5757,12 @@
|
|||
"url": "/_mozilla/mozilla/textcontent.html"
|
||||
}
|
||||
],
|
||||
"mozilla/timer_eventInvalidation.html": [
|
||||
{
|
||||
"path": "mozilla/timer_eventInvalidation.html",
|
||||
"url": "/_mozilla/mozilla/timer_eventInvalidation.html"
|
||||
}
|
||||
],
|
||||
"mozilla/title.html": [
|
||||
{
|
||||
"path": "mozilla/title.html",
|
||||
|
|
21
tests/wpt/mozilla/tests/mozilla/timer_eventInvalidation.html
Normal file
21
tests/wpt/mozilla/tests/mozilla/timer_eventInvalidation.html
Normal file
|
@ -0,0 +1,21 @@
|
|||
<html>
|
||||
<head>
|
||||
<script src="/resources/testharness.js"></script>
|
||||
<script src="/resources/testharnessreport.js"></script>
|
||||
</head>
|
||||
<body>
|
||||
<script>
|
||||
async_test(function(test) {
|
||||
// couldn't reproduce this behaviour without iframe
|
||||
var iframe = document.createElement('iframe');
|
||||
window.addEventListener('test_done', function() {
|
||||
// if event invalidation is not properly implemented
|
||||
// servo will crash during a short period of time
|
||||
setTimeout(() => test.done(), 50);
|
||||
}, false);
|
||||
iframe.src = 'timer_eventInvalidation_test.html';
|
||||
document.body.appendChild(iframe);
|
||||
});
|
||||
</script>
|
||||
</body>
|
||||
</html>
|
|
@ -0,0 +1,14 @@
|
|||
<html>
|
||||
<head>
|
||||
</head>
|
||||
<body>
|
||||
<script>
|
||||
setTimeout(function() {
|
||||
setTimeout(function() {
|
||||
var e = new Event('test_done');
|
||||
window.parent.dispatchEvent(e);
|
||||
}, 0);
|
||||
}, 0);
|
||||
</script>
|
||||
</body>
|
||||
</html>
|
Loading…
Add table
Add a link
Reference in a new issue