scratchfoundation/scratch-vm
1
0
Fork 0
mirror of https://github.com/scratchfoundation/scratch-vm.git synced 2024-12-24 23:12:24 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Make _removeThread safe during thread iteration

Browse source 
Before: `_removeThread` stops a thread and immediately removes it from
the runtime's thread array. If this happens while iterating over the
thread array, such as in the case of clone deletion, some elements of
the array can be missed.

After: `_removeThread` has been renamed to `_stopThread`. It still stops
the thread immediately but it no longer removes the thread from the
runtime's thread array. Instead, `_stopThread` marks the thread for
later removal. Such threads are removed at the top of the next `_step`.
This commit is contained in:
Christopher Willis-Ford 2017-11-17 12:12:07 -08:00
parent 8ce20fd308
commit b19af399fa
1 changed files with 12 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
src/engine/runtime.js
View file

@ -786,17 +786,14 @@ class Runtime extends EventEmitter {
} }
/** /**
* Remove a thread from the list of threads. * Stop a thread: stop running it immediately, and remove it from the thread list later.
* @param {?Thread} thread Thread object to remove from actives * @param {!Thread} thread Thread object to remove from actives
*/ */
_removeThread (thread) { _stopThread (thread) {
// Mark the thread for later removal
thread.isKilled = true;
// Inform sequencer to stop executing that thread. // Inform sequencer to stop executing that thread.
this.sequencer.retireThread(thread); this.sequencer.retireThread(thread);
// Remove from the list.
const i = this.threads.indexOf(thread);
if (i > -1) {
this.threads.splice(i, 1);
}
} }
/** /**
@ -859,7 +856,7 @@ class Runtime extends EventEmitter {
// edge activated hat thread that runs every frame // edge activated hat thread that runs every frame
continue; continue;
} }
this._removeThread(this.threads[i]); this._stopThread(this.threads[i]);
return; return;
} }
} }
@ -1035,8 +1032,7 @@ class Runtime extends EventEmitter {
continue; continue;
} }
if (this.threads[i].target === target) { if (this.threads[i].target === target) {
this.threads[i].isKilled = true; this._stopThread(this.threads[i]);
this._removeThread(this.threads[i]);
} }
} }
} }
@ -1075,11 +1071,7 @@ class Runtime extends EventEmitter {
} }
this.targets = newTargets; this.targets = newTargets;
// Dispose all threads. // Dispose all threads.
const threadsCopy = this.threads.slice(); this.threads.forEach(thread => this._stopThread(thread));
while (threadsCopy.length > 0) {
const poppedThread = threadsCopy.pop();
this._removeThread(poppedThread);
}
} }
/** /**
@ -1093,6 +1085,10 @@ class Runtime extends EventEmitter {
} }
this.profiler.start(stepProfilerId); this.profiler.start(stepProfilerId);
} }
// Clean up threads that were told to stop during or since the last step
this.threads = this.threads.filter(thread => !thread.isKilled);
// Find all edge-activated hats, and add them to threads to be evaluated. // Find all edge-activated hats, and add them to threads to be evaluated.
for (const hatType in this._hats) { for (const hatType in this._hats) {
if (!this._hats.hasOwnProperty(hatType)) continue; if (!this._hats.hasOwnProperty(hatType)) continue;