Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential race condition in "readBlobAsText / readBlobAsArrayBuffer" ? #353

Closed
HolgerMoser opened this issue Jun 17, 2016 · 4 comments
Closed

Comments

@HolgerMoser
Copy link

HolgerMoser commented Jun 17, 2016

I encountered an issue with safari (very hard to reproduce) where the promise in fileReaderReady (fetch.js) is never resolved or rejected. The code that triggers this issue looks like this:

fetch(someUrl, someRequest).then((response) => { response.json().then(json) { // this is never reached... }  })

This happens very rarely, however once the (safari) browser is in that state, it only helps to close the current tab / open a new one / reopen the whole browser. Using the debugger i found that execution chain breaks at the following location:

function fileReaderReady(reader) {
    return new Promise(function(resolve, reject) {
      reader.onload = function() {
        resolve(reader.result)                   <-- never executed
      }
      reader.onerror = function() {
        reject(reader.error)                       <-- never executed
      }
    })
  }

  function readBlobAsText(blob) {
    var reader = new FileReader()
    reader.readAsText(blob)
    return fileReaderReady(reader)      <-- this function is executed
  }

I can see in the debugger that the FileReader object has read data successfully from the backend and that its in the "successfully read all data" state, so nothing unusual here. Using another tab at the same time / another browser works (so the backend has nothing to do with this problem).

Now for my question:

I would expect readBlobAsText to first register the events "onload" / "onerror" on the reader object before triggering the read call (readAsText). All the code examples i found also do it that way, however (under "normal" circumstances) the current code seems to work just fine. So:

  • Is it possible that this is a race condition? (the fact that it either always works or never works speaks against it)
  • Could this explain the strange bug i encountered? (as i said its very hard to reproduce, so far impossible to reproduce on purpose .. i tried adding some wait cycles between the two calls and it still worked in the normal case)

Maybe someone with more knowledge of the inner workings of the browser can shed some light on this. Regards

@mislav
Copy link
Contributor

mislav commented Jun 17, 2016

There was a problem in Safari when a network error could lead to the promise never being resolved. We fixed it with 619ee08 by handling the ontimeout callback. But your report doesn't seem like the same issue. Just in case, please ensure that you're running the latest version of the library.

I'm not sure what causes your problem. We don't have very much insight into the inner working of the Safari browser. It would really help a lot of the problem was consistently reproducible. Also, have you searched the web for other reports of people having issues with FileReader never invoking its callbacks?

@HolgerMoser
Copy link
Author

I already searched the net for this issue, without much success. The "ontimeout callback issue" does not seem to be related to the issue i encountered, but i will update to the latest version and see if it helps, thanks.

Im also currently testing the following change (this is how i would expect the function to work):

function readBlobAsText(blob) {
    var reader = new FileReader()
    var promise = fileReaderReady(reader)
    reader.readAsText(blob)
    return promise
  }

@mislav
Copy link
Contributor

mislav commented Jun 20, 2016 via email

@Qowyn
Copy link

Qowyn commented Aug 23, 2016

After reading the FileAPI spec this clearly looks like a race condition. I also consider it bad practice to attach event handlers that should handle something after that something already started. It's like expecting to recieve all click events that allready happend on a Node upon attaching a handler to onclick.

@mislav mislav closed this as completed in 08e6a85 Nov 14, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants