On Thu Sep 12 13:15:53 2024 +0000, Jacek Caban wrote:
That's because those tests first create two async XHRs and then a sync
XHR. It appears that gecko will simply hang if we don't process the async XHR tasks in the sync XHR message loop… I guess it's about use of tasks in `nsio.c`. In general, it's not tasks themselves that we need to block here, but rather script (or host) visible notifications. We don't want to stall other bindings during sync XHR just like we don't want to stop repainting GUI. What we want is to give the script an impression that it's not processed. And yeah, such synchronous operations in a middle of otherwise asynchronous environment are broken by design, that's why sync XHRs are deprecated. That said, it's probably fine if we don't get it exactly right.
Yeah, what I mean is that we're supposed to block tasks (the point of the MR) if we're in a nested message loop. This works fine without sync XHRs. For sync XHRs, Gecko uses a message loop and expects the tasks to be processed (probably the ones you said in `nsio.c`).
Now what if a nested message loop (that should block all tasks!) calls sync XHR send() at some point? It has to process the tasks, else Gecko hangs. So, I took the "safe" approach and process all tasks in such case—i.e. behavior as before this MR (I set `tasks_locked` to 0 during sync XHR send, then back to its previous value; nested message loops within sync XHR send will still not get processed and that's fine).
Granted, I doubt it will happen in practice, so it's probably good enough, but if it does we can probably revisit it. We should probably keep it simple for now.
BTW note that it's more than just the sync XHR io tasks themselves. As I said, even tasks related to prior async XHRs in the xhr.js tests had to be processed for some reason (which have nothing to do with the sync XHR), else it hanged. That's why I gave up trying to "only process tasks related to sync XHR" (IO or otherwise) which was my idea originally that failed, and went with something simpler for such a niche case.