Async function blocks other threads from executing?

evandarcy commented 3 years ago

Calling a function like this in two separate threads will result in the second thread being blocked from running until first thread callback is hit.

var wrapper = async function(time, callback){`
    console.log('start');
    setTimeout(function(){
        console.log('end');
        callback('hit');
    }, time);
};
interpreter.setProperty(globalObject, 'await',
    interpreter.createAsyncFunction(wrapper));

If I call await(2000) in both threads and run them simultaneously, I expect to see 'start' printed simultaneously. Instead the first 'start' will be printed followed by 'end' and then the second 'start' will be printed.

Should it be possible in theory to achieve this? If so, any suggestions are appreciated! Perhaps I am approaching this from the wrong angle or there is a workaround I have not considered.

Thanks!

cpcallen commented 3 years ago

I think that get a proper answer to your question you will need to be more specific about what you mean by "threads". JS-Interpreter is single-threaded (like any other normal JavaScript runtime is if you're not using Web Workers). How are you creating these 'threads'—by calling appendCode? That just adds more code to be executed to the end of the (one and only) thread.

Functions created by createAsyncFunction are only asynchronous from the point of view of the world outside the Interpreter; from the point of view of the code being interpreted they are ordinary synchronous functions. Your await(2000) calls are, therefore, just calls to a function that happen to take 2s to complete, blocking any other execution until they are finished.

There is a way to have multiple threads running 'in parallel' by creating multiple Interpreter instances; it looks like that might be what you want to do. (If that is what you are doing and things are still not working as desired then please provide a complete working example demonstrating the issue.)

Alternatively, have you considered just having the interpreted code call the (interpreted) setTimeout function in the usual way?

As an aside:

var wrapper = async function(time, callback){

The async here is certainly misplaced. Fortunately it has no effect (because the return value of the anonymous function(time, callback) is discarded by the interpreter, so the fact that it is unexpectedly a Promise does no harm)—but it suggests that there has been a fundamental misunderstanding of how createAsyncFunction works. (It is worth noting that createAsyncFunction predates ES6 and the advent of Promise, and considerably predates ES2017's async/await.)

evandarcy commented 3 years ago

Hi @cpcallen , really appreciate the quick, detailed response!

My apologies, when I said 'thread', I was referring to threads as in multiple 'parallel' Interpreter instances as shown in the JS-Interpreter Thread Demo as you mentioned.

This is the code I am testing with, which I based off the above demo. I modified it so that the code is run when the run button is pressed instead of having to click parse and step. continously.

<html>
<head>
  <meta charset="utf-8">
  <title>JS-Interpreter Thread Demo</title>
  <script src="https://neil.fraser.name/software/JS-Interpreter/acorn.js"></script>
  <script src="https://neil.fraser.name/software/JS-Interpreter/interpreter.js"></script>
  <script>
    function initAlert(interpreter, globalObject) {
      var wrapper = function(text) {
        console.log(text);
        return;
      };
      interpreter.setProperty(globalObject, 'alert',
          interpreter.createNativeFunction(wrapper));

      var wrapper = function(time, callback){
          console.log('start');
          setTimeout(function(){
            console.log('end');
            callback('hit');
          }, time);
      };
      interpreter.setProperty(globalObject, 'await',
          interpreter.createAsyncFunction(wrapper));
    }

    // Create an interpreter whose global scope will be the cross-thread global.
    var myInterpreter = new Interpreter('', initAlert);
    var threads = [];
    var runInterval;

    function parseButton(n) {
      var code = document.getElementById('code' + n).value;
      var intrp = new Interpreter('');
      // Replace this thread's global scope with the cross-thread global.
      intrp.stateStack[0].scope = myInterpreter.globalScope;
      intrp.appendCode(code);
      threads[n] = intrp.stateStack;
    }

    function runButton(n) {
        parseButton(1);
        parseButton(2);

        if(runInterval) clearInterval(runInterval)
        runInterval = setInterval(run, 5);
        var threadFinishedStatus = [ false, false ];

        function run(){

            for(let i =1; i<=2; i++){
                myInterpreter.stateStack = threads[i];
                try {
                    var ok = myInterpreter.step();
                    threadFinishedStatus[ i - 1 ] = false;
                } finally {
                    if (!ok) {
                      threadFinishedStatus[ i - 1 ] = true;
                    }
                }
            }
            if (!threadFinishedStatus.includes(false)) {
                clearInterval( runInterval );
                runInterval = null;
                console.log('finished');
            }
        }
    }

  </script>
</head>
<body>
  <table style="width: 100%;">
    <tr>
      <td>
        Thread #1
      </td>
      <td>
        Thread #2
      </td>
    </tr>
    <tr>
      <td>
  <textarea id="code1" style="height: 6em; width: 100%;">
  await(2000);
  </textarea>
      </td>
      <td>
  <textarea id="code2" style="height: 6em; width: 100%;">
    await(2000);
  </textarea>
      </td>
    </tr>
  </table>

  <button onclick="runButton()" id="runButton">Run</button>
</body>
</html>

The goal is to be able to run two async programs in each thread independently of each other.

cpcallen commented 3 years ago

Ah hah: this is actually a documentation bug!

The error is here, in code that you have copied verbatim from thread.html (paraphrased for clarity):

    // Create an interpreter whose global scope will be the cross-thread global.
    var myInterpreter = new Interpreter('', initAlert);
    var threads = [];

    function parseButton(n) {
      var code = document.getElementById('code' + n).value;
      var intrp = new Interpreter('');
      // Replace this thread's global scope with the cross-thread global.
      intrp.stateStack[0].scope = myInterpreter.globalScope;
      intrp.appendCode(code);
      threads[n] = intrp.stateStack;
    }

    function stepButton(n) {
      myInterpreter.stateStack = threads[n];
      var ok = myInterpreter.step();
    }

The problem is that Interpreter instances have state other than their .stateStack—in particular, they have a .paused_ boolean which is used to manage async functions. When thread[1] calls await, myInterpreter.paused_ is set to true; this has the unintended effect of pausing both threads.

@NeilFraser: not sure what the preferred fix is here. Options:

Keep a per-thread array of .paused_ values, copying them to/from myInterpreter.paused_ as each thread is scheduled.
Have threads be an array of Interpreter instances , each fully encapsulating their own state (except for the shared global scope object that they have transplanted in at creation time).

The latter seems considerably cleaner overall; the resulting code would look something like:

    // Create an interpreter whose global scope will be the cross-thread global.
    var myInterpreter = new Interpreter('', initAlert);
    var threads = [];

    function parseButton(n) {
      var code = document.getElementById('code' + n).value;
      var intrp = new Interpreter('');
      // Replace this thread's global scope with the cross-thread global.
      intrp.stateStack[0].scope = myInterpreter.globalScope;
      intrp.appendCode(code);
      threads[n] = intrp;
    }

    function stepButton(n) {
      var ok = threads[n].step();
    }

evandarcy commented 3 years ago

Apologies, I accidentally closed the issue.

@cpcallen ,you're a lifesaver, there is not way I would have figured that out by myself! I implemented your change and it is working as expected now. Thanks again!

NeilFraser / JS-Interpreter

Async function blocks other threads from executing? #196