naja icon indicating copy to clipboard operation
naja copied to clipboard

Race hazard in submitForm call

Open janturon opened this issue 2 years ago • 1 comments

Bug Report

Naja 2.4.0, UIHandler.ts:127 (and the clickForm above) should not be declared async. It should return a Promise from makeRequest synchronously (there is no await in the method body). Async call lead to race hazard. In our case where Naja is bundled with other scripts (so the processing is a bit delayed), well defined input to submitForm is undefined in the UIHandler, as the submitForm is called in another thread due to the async. The issue appeared on Safari in minified code only.

There can be other thread safety issues: I suggest to checkout scenarios where Naja is not called in the main thread.

Code that reproduces the problem

N/A (race hazard in bundled code)

janturon avatar Jul 12 '22 07:07 janturon

Hi, I'm afraid I don't fully understand.

The methods are marked async so that they consistently return a Promise. Even though they don't await anything, they use early-return and throw which, in an async function, are translated to promise resolution and rejection, respectively.

Also, JavaScript is inherently single-threaded (well, unless you are using some kind of Workers or Worklets, but I presume you would have mentioned that). Yes, "async" code is delayed to the microtask queue, so there might be some odd timing issues, but:

If Safari implements async functions according to the spec, the body of the function runs synchronously all the way to the first await expression – and in this case, as you've pointed out, there is none, i.e. the whole function should actually already be executed synchronously, as you suggest.

I know it might be difficult, but I'd really appreciate a reproducible example. I have Safari at my disposal so no problem if it's just one of its quirks. Without that, I'm afraid I won't be able to help you :(

jiripudil avatar Jul 12 '22 14:07 jiripudil

Hi, I'm closing this for inactivity. Feel free to reopen this issue if you have further insights into the problem.

jiripudil avatar Sep 25 '22 12:09 jiripudil