evaluates block each other

[TelephoneTan]

private suspend fun evalAndAwait(evalBlock: suspend () -> Any?): Any? {
        ensureNotClosed()
        evalException = null
        loadModules()
        val result = jsResultMutex.withLock {
            jsMutex.withLock { evalBlock() }
            awaitAsyncJobs()
            jsMutex.withLock { getEvaluateResult(context, globals) }
        }
        handleException()
        return result
    }

I have a JS call which will be delayed for 5 seconds, within the 5 second other evaluate calls block. I think it is the jsResultMutex here blocks other evaluate calls.

[TelephoneTan]

please fix this

[dokar3]

You can't have multithreaded JavaScript here, that's how it's designed. Running them simultaneously will break the runtime (stack overflows, async jobs, and results would make it a nightmare).

If concurrency is needed please use multiple runtimes.

[TelephoneTan]

But this is not multithreading, all evaluates are on the same thread(I use a single thread dispatcher to invoke all evaluates).

They just suspend and but the mutex block them.

This is just like in JavaScript I can have both a timeout pending and a lot of work processing, they won't block each other because JavaScript is single-thread but this is just asynchronized.

[dokar3]

This is weird, if all calls happen on the same thread, you shouldn't have a chance to start another evaluate call when the jobs from the previous evaluate are still running, it might be a bug.

Is there any sample code to elaborate on this?

[TelephoneTan]

That is what I mean.

In quickjs we can call JS_Eval multiple times, and handle all pending jobs at once, they won't block each other, this is asynchronise.

But in this library we can't do this because evaluates block each other, this is actually synchronise and it differs from the JS world.

[TelephoneTan]

Maybe we can move all the global variables used by evaluate now into an evaluate_context, and bind an evaluate_context to each evaluate call so that different evaluate calls share nothing and they can run concurrently.

Note that this doesn't mean the underlying JS runs cocurrently on multithreads, they are still on single thread(just use a single-thread dispatcher to run the JS code), we just bring asynchronise to the kotlin layer(evaluate).

[dokar3]

I kind of understand, do you mean using "evaluate" to create a promise but not using "await" on it and then doing the same thing? This is not currently possible because evaluate will simply wait for all jobs it creates.

Managing an evaluate level globals is possible, it would probably require a lot of changes, so I won't say if it will come or come at some time.

[TelephoneTan]

No, I don't mean not waiting for its jobs, evaluate still waits for its jobs, this is ok and it won't block the other evaluates because join is suspendable in kotlin.

The root cause why currently evaluate blocks the other evaluates is it uses a "result mutex" internally to wrap all the code.

I have checked the code, I infer that the "result mutex" is here to protect the "result" global variable. Sure, if evaluate uses a global variable to store its result, two evaluates should never run concurrently because there is only one global variable and it will cause data race.

So this is why I suggest to move all the global variables into a context that bound to the evaluate call so that we can remove the global-variable-guarding mutex that blocks multiple evaluate calls.

[dokar3]

Okay, I still didn't get it, Any sample code to elaborate on the block?

[TelephoneTan]

val singleThreadDispatcher : CoroutineDispatcher = TODO()
val scope = CoroutineScope(singleThreadDispatcher)
val q = QuickJS.create(Dispatcher.DEFAULT)
scope.launch{
   println(q.evaluate(TODO("some js code that calls delay asyncFunction then returns a string")))
}
scope.launch{
   println(q.evaluate(TODO("some js code that just returns a string immediately")))
}

The first one delays for 5 seconds, and the second one should print immediately.

But the first one blocks the second one, so the second one prints only after the first one prints.

[dokar3]

But the first one blocks the second one, so the second one prints only after the first one prints.

Okay, this is the expected behavior, right?

[TelephoneTan]

But the first one blocks the second one, so the second one prints only after the first one prints.

Okay, this is the expected behavior, right?

Yes, this is the correct behaviour currently, it's not a bug.

But this behaviour is not asynchronized, the pure sync JS code being executing in sequence is okay, but if the JS code in one evaluate invokes an async function and is suspended, it should yields the execution to the sync JS code of another evaluate.

In JS world, calling async function will not block the whole world, it's just a Promise.

[dokar3]

Sorry for the delay, do you mean managing multiple evaluate (and their event loop) in the same coroutine dispatcher?

[TelephoneTan]

I mean that if one evaluate suspends at asyncFunction invoke, another evaluate should have a chance to run.

This does not contradict single-thread, two evaluates can run on different coroutines that are dispatched by a single thread dispatcher.

Just like you can not call Thread.sleep() on main thread in java, it will block the main thread and freeze the app, but you can call delay using Dispatcher.Main in kotlin, this will not block the main thread because the coroutine are just suspended.

Likewise, two evaluates on different coroutines but in the same thread should not block each other, if one calls async function and pending, it should just be suspended and yield the chance to run to another one.

[TelephoneTan]

Calling asyncFunction is just like new Promise() in JS, it should not block other JS code to run.

[dokar3]

I got your points, suspending is not blocking and we can 'await' multiple evaluate in a single thread dispatcher. However, managing multiple evaluate and their loop separately is not quite easy, so I wouldn't say I will make this change at any time, mostly because I haven't had much time to work on it these days.

[TelephoneTan]

Maybe I can help with it.

I think just move all global states into a context and remove the result mutex, then everything will be fine.

[dokar3]

Great, would be glad to see that!