Fix stopping a thread from inside of its blocks

Works in command or reporter after yielding or not. These snippets are covered by the tests: thread.status = Thread.STATUS_DONE; thread.status = 4; (indirectly) sequencer.retireThread(thread);
TurboWarp · May 15, 2024 · f548d1b · f548d1b
1 parent 73d71c0
commit f548d1b
Show file tree

Hide file tree

Showing 5 changed files with 206 additions and 4 deletions.
diff --git a/src/compiler/jsexecute.js b/src/compiler/jsexecute.js
@@ -163,7 +163,7 @@ const executeInCompatibilityLayer = function*(inputs, blockFunction, isWarp, use
         return returnValue;
     }
 
-    if (thread.status === 1 /* STATUS_PROMISE_WAIT */) {
+    if (thread.status === 1 /* STATUS_PROMISE_WAIT */ || thread.status === 4 /* STATUS_DONE */) {
         // Something external is forcing us to stop
         yield;
         // Make up a return value because whatever is forcing us to stop can't specify one
@@ -190,14 +190,12 @@ const executeInCompatibilityLayer = function*(inputs, blockFunction, isWarp, use
             return returnValue;
         }
 
-        if (thread.status === 1 /* STATUS_PROMISE_WAIT */) {
+        if (thread.status === 1 /* STATUS_PROMISE_WAIT */ || thread.status === 4 /* STATUS_DONE */) {
             yield;
             return finish('');
         }
     }
 
-    // todo: do we have to do anything extra if status is STATUS_DONE?
-
     return finish(returnValue);
 }`;
 

diff --git a/src/engine/execute.js b/src/engine/execute.js
@@ -579,6 +579,9 @@ const execute = function (sequencer, thread) {
                     parentValues[inputName] = primitiveReportedValue;
                 }
             }
+        } else if (thread.status === Thread.STATUS_DONE) {
+            // Nothing else to execute.
+            break;
         }
     }
 

diff --git a/src/engine/sequencer.js b/src/engine/sequencer.js
@@ -237,6 +237,9 @@ class Sequencer {
             } else if (thread.status === Thread.STATUS_YIELD_TICK) {
                 // stepThreads will reset the thread to Thread.STATUS_RUNNING
                 return;
+            } else if (thread.status === Thread.STATUS_DONE) {
+                // Nothing more to execute.
+                return;
             }
             // If no control flow has happened, switch to next block.
             if (thread.peekStack() === currentBlockId && !thread.peekStackFrame().waitingReporter) {

diff --git a/test/fixtures/tw-block-stop-thread.sb3 b/test/fixtures/tw-block-stop-thread.sb3
diff --git a/test/integration/tw-block-stop-thread.js b/test/integration/tw-block-stop-thread.js
@@ -0,0 +1,198 @@
+const {test} = require('tap');
+const fs = require('fs');
+const path = require('path');
+const VM = require('../../src/virtual-machine');
+const Thread = require('../../src/engine/thread');
+const Clone = require('../../src/util/clone');
+
+const fixturePath = path.join(__dirname, '../fixtures/tw-block-stop-thread.sb3');
+const fixtureData = fs.readFileSync(fixturePath);
+
+const stopByRetireThread = thread => {
+    thread.target.runtime.sequencer.retireThread(thread);
+};
+
+const stopBySetStatus = thread => {
+    thread.status = Thread.STATUS_DONE;
+};
+
+test('constants', t => {
+    t.equal(Thread.STATUS_DONE, 4);
+    t.end();
+});
+
+for (const [enableCompiler, stopFunction] of [
+    [true, stopByRetireThread],
+    [true, stopBySetStatus],
+    [false, stopByRetireThread],
+    [false, stopBySetStatus]
+]) {
+    const subtestName = `${enableCompiler ? 'compiler' : 'interpreter'} - ${stopFunction.name}`;
+
+    test(`${subtestName} - stop in command block`, t => {
+        const vm = new VM();
+        vm.setCompilerOptions({enabled: enableCompiler});
+        t.equal(vm.runtime.compilerOptions.enabled, enableCompiler);
+
+        const callOrder = [];
+        vm.addAddonBlock({
+            procedureCode: 'first block %s',
+            arguments: ['number or text'],
+            callback: (args, util) => {
+                callOrder.push(['first block', Clone.simple(args)]);
+                stopFunction(util.thread);
+            }
+        });
+        vm.addAddonBlock({
+            procedureCode: 'inner block',
+            return: 1,
+            callback: args => {
+                callOrder.push(['input block', Clone.simple(args)]);
+                return callOrder.length;
+            }
+        });
+        vm.addAddonBlock({
+            procedureCode: 'second block',
+            callback: args => {
+                callOrder.push(['second block', Clone.simple(args)]);
+            }
+        });
+
+        vm.loadProject(fixtureData).then(() => {
+            vm.greenFlag();
+            vm.runtime._step();
+            t.same(callOrder, [
+                ['input block', {}],
+                ['first block', {'number or text': 1}]
+            ]);
+            t.end();
+        });
+    });
+
+    test(`${subtestName} - stop in command block after yielding once`, t => {
+        const vm = new VM();
+        vm.setCompilerOptions({enabled: enableCompiler});
+        t.equal(vm.runtime.compilerOptions.enabled, enableCompiler);
+
+        const callOrder = [];
+        vm.addAddonBlock({
+            procedureCode: 'first block %s',
+            arguments: ['number or text'],
+            callback: (args, util) => {
+                callOrder.push(['first block', Clone.simple(args)]);
+                if (callOrder.length === 1) {
+                    util.yield();
+                } else {
+                    stopFunction(util.thread);
+                }
+            }
+        });
+        vm.addAddonBlock({
+            procedureCode: 'inner block',
+            return: 1,
+            // Ignore calls because interpreter will re-evaluate on yield but compiler won't
+            callback: () => 5
+        });
+        vm.addAddonBlock({
+            procedureCode: 'second block',
+            callback: args => {
+                callOrder.push(['second block', Clone.simple(args)]);
+            }
+        });
+
+        vm.loadProject(fixtureData).then(() => {
+            vm.greenFlag();
+            vm.runtime._step();
+            t.same(callOrder, [
+                ['first block', {'number or text': 5}],
+                ['first block', {'number or text': 5}]
+            ]);
+            t.end();
+        });
+    });
+
+    test(`${subtestName} - stop in reporter block`, t => {
+        const vm = new VM();
+        vm.setCompilerOptions({enabled: enableCompiler});
+        t.equal(vm.runtime.compilerOptions.enabled, enableCompiler);
+
+        const callOrder = [];
+        vm.addAddonBlock({
+            procedureCode: 'first block %s',
+            arguments: ['number or text'],
+            callback: args => {
+                callOrder.push(['first block', Clone.simple(args)]);
+            }
+        });
+        vm.addAddonBlock({
+            procedureCode: 'inner block',
+            return: 1,
+            callback: (args, util) => {
+                callOrder.push(['input block', Clone.simple(args)]);
+                stopFunction(util.thread);
+                return callOrder.length;
+            }
+        });
+        vm.addAddonBlock({
+            procedureCode: 'second block',
+            callback: args => {
+                callOrder.push(['second block', Clone.simple(args)]);
+            }
+        });
+
+        vm.loadProject(fixtureData).then(() => {
+            vm.greenFlag();
+            vm.runtime._step();
+            t.same(callOrder, [
+                ['input block', {}]
+            ]);
+            t.end();
+        });
+    });
+
+    test(`${subtestName} - stop in reporter block after yielding once`, t => {
+        const vm = new VM();
+        vm.setCompilerOptions({enabled: enableCompiler});
+        t.equal(vm.runtime.compilerOptions.enabled, enableCompiler);
+
+        const callOrder = [];
+        vm.addAddonBlock({
+            procedureCode: 'first block %s',
+            arguments: ['number or text'],
+            callback: args => {
+                callOrder.push(['first block', Clone.simple(args)]);
+            }
+        });
+        vm.addAddonBlock({
+            procedureCode: 'inner block',
+            return: 1,
+            callback: (args, util) => {
+                callOrder.push(['input block', Clone.simple(args)]);
+                if (callOrder.length === 1) {
+                    util.yield();
+                    // TODO: interpreter bug, should not need to do this...
+                    util.thread.peekStackFrame().waitingReporter = true;
+                } else {
+                    stopFunction(util.thread);
+                }
+                return callOrder.length;
+            }
+        });
+        vm.addAddonBlock({
+            procedureCode: 'second block',
+            callback: args => {
+                callOrder.push(['second block', Clone.simple(args)]);
+            }
+        });
+
+        vm.loadProject(fixtureData).then(() => {
+            vm.greenFlag();
+            vm.runtime._step();
+            t.same(callOrder, [
+                ['input block', {}],
+                ['input block', {}]
+            ]);
+            t.end();
+        });
+    });
+}