Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

fix: remove loading race conditions #1

Merged
merged 1 commit into from
Aug 7, 2022

Conversation

jeswr
Copy link
Contributor

@jeswr jeswr commented Aug 7, 2022

Uses async/await to avoid race conditions. Making the fetches in-order doesn't noticably slow down anything so no point trying to make just those parts async for now.

@josd josd merged commit ae26d57 into eyereasoner:master Aug 7, 2022
@jeswr jeswr deleted the fix/race-conditions branch August 7, 2022 13:06
@Jean-Luc-Picard-2021
Copy link

Jean-Luc-Picard-2021 commented Aug 7, 2022

You didn’t fix some “race condition” or whatever. I never wanted to
point out a “race condition” of some sort. And with this code the
fetches are still in-order, since “await” does what the word says,

and "async" doesn't mean "par", i.e. parallel:

SWIPL(Module).then(async (module) => {
    await fetchWrite('<path>/eye.pl', '/eye.pl');
    await fetchWrite('<path>/reasoning/socrates/socrates.n3', 'socrates.n3');
    await fetchWrite('<path>/reasoning/socrates/socrates-query.n3', 'socrates-query.n3');

You totally misundertood my post. I was only talking about different
coding styles. Now you switched from coding style Promise chain
to coding style async/await. But it still sequential und not parallel loading.

But this is fine that it is sequential, I do not bother whether it is sequential
or not, since the SWI-Prolog thread scope was SWI-Prolog WASM
and not eyebrow. But anyway I am happy that you could solve another

ordering problem.

@Jean-Luc-Picard-2021
Copy link

Jean-Luc-Picard-2021 commented Aug 7, 2022

For your problem of a "race condition", if you want to load in parallel, I guess
you need to find the equivalent of create_task() and gather() from Python asyncio
library. You need to find the equivalent somewhere in the bowels of JavaScript.

See also here:

Multitasking with asyncio
A task is a kind of coroutine. A coroutine can stop in the
middle of some code. When the coroutine is called again,
it starts where it left off. A coroutine is declared with the
keyword async, and the keyword await indicates that
the coroutine is giving up control at that point.

async def main():
    animation_controls = AnimationControls()
    button_task = asyncio.create_task(monitor_button(button_pin, animation_controls))
    animation_task = asyncio.create_task(rainbow_cycle(animation_controls))
    blink_task = asyncio.create_task(blink(animation_controls))
    # This will run forever, because no tasks ever finish.
    await asyncio.gather(button_task, animation_task, blink_task)

https://learn.adafruit.com/adafruit-esp32-s2-feather/multitasking-with-asyncio

@Jean-Luc-Picard-2021
Copy link

Jean-Luc-Picard-2021 commented Aug 8, 2022

The multiple Promise chain fetches did indeed run in “parallel”. At least
they would post multiple Promises to the operating system task queue of
the browser. And when ever one of the Promises is finished, it would

call their then() part, in first come first serve order. On the other hand
the new code with async/await does process the fetches sequentially,
since the await will await each fetch to be finished.

Whether there is some “race condition” in the parallel case I don’t
know. It depends how functional without other side effects things
like Module.FS.writeFile() work, and whether you mean a

“race condition” with the rest, like pl("format('SWI-Prolog WebAssembly
ready!~n')"); etc… Or you think there is not enough opportunity
for the involved operating systems and the browser to do the requests

in true parallel, which might or might not be true, this depends on a
quite a number of factors and probably deserves practical testing. If
you want to execute the fetch in parallel, and if you want to join

the completion of the fetches, you could use:

How to do multiple fetch requests in parallel using JavaScript?
https://melvingeorge.me/blog/do-multiple-fetch-requests-parallel-javascript

The above suggests Promise.all() from JavaScript. But mostlikely
this is not the same as asyncio.gather() from Python, since the later
does also schedule new tasks.

@jeswr
Copy link
Contributor Author

jeswr commented Aug 8, 2022

Apologies for the lazy PR message - I was doing this in a bit of a rush.

Indeed the undeterministic order of the Module.FS.writeFile() and SWIPL(module).then... operations was what I was trying to get rid of; as the fact that the page was behaving differently on different loads was indicating race conditions.

On the parallel fetch; with anything like that you can always just run the fetches and resolve them later like below, no need for promise.all.

The only reason I didn't do this in the PR is because the site loads in a negligible amount of time so there was no point in making those kinds of changes to keep the fetch calls in 'parallel'.

// Start fetches in 'parallel'
const unresolvedFetch1 = fetch(`http://example.org#1`)
const unresolvedFetch2 = fetch(`http://example.org#2`)

doTaskWithResponse(await unresolvedFetch1);
doTaskWithResponse(await unresolvedFetch2);

Sorry for any confusion with my lazy use of terminology around changes in syntax/syntaxtic sugar vs. changes in behavior.

@Jean-Luc-Picard-2021
Copy link

Jean-Luc-Picard-2021 commented Aug 8, 2022

Ok, I only wanted to make sure that nothing got broken by some rush.
Thats the reason why I was chewing on the issue by some further posts here.
But I guess I can set free this issue, taking home a glimps of eyebrow.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants