Repository: pyodide/pyodide
Pull request: Fix package loading race condition
There was a fairly hard-to-reproduce race condition in how packages are loaded.

After the full set of packages to load are resolved, each of them is "loaded" by adding `<script>` elements to the end of the document.  When these package `.js` files are run, the first thing they do is increment a global counter, and call the `monitorRunDependencies` callback.  When each is done, that counter is decremented, and the same `monitorRunDependencies` callback is called.  Our `monitorRunDependencies` callback assumes everything is done (and the Python is ready to call) when that counter reaches 0.  However, in rare cases, the first few packages load and finish before all of the packages have started running, i.e., the counter follows a pattern like "1 2 3 4 5 4 3 2 1 0 1 2 1 0".  There doesn't seem to be a good way to do this reliably given how emscripten's API around this works, so the solution, since we know exactly how many packages we expect to load, is to just wait for the callback to be called exactly `npackages * 2` times.

I'm hoping this will resolve some of the random failures we've seen in CI.

Thanks @acmiyaguchi for finding the bug and helping me find the solution.

Fixes #270.

Problem statement:
async imports don't play nicely with non-async ones
A Mozillian found an aspect of async importing that appears to not work. The code block below demonstrates the issue - it appears that mixing imports that fetch resources (other stuff from the pyodide stack) don't play very nicely with imports that don't.

You can replace the `from js import document` line with some other expected-to-work import statement that uses python and you get the same error, so it appears that this issue is caused by the async stuff in some way.

```
%%py
import matplotlib.pyplot as plt
from js import document
```

Hints from issue discussion:
UPDATE: we're having trouble reproducing the bug all of a sudden, so I'm not sure if there's a network issue w/ where pyodide assets are being served. We get errors that look like:

```
Error: Traceback (most recent call last):
  File "/lib/python3.7/site-packages/pyodide.py", line 35, in eval_code
    exec(compile(mod, '<exec>', mode='exec'), ns, ns)
  File "<exec>", line 1, in <module>
  File "/lib/python3.7/site-packages/pandas/__init__.py", line 19, in <module>
    "Missing required dependencies {0}".format(missing_dependencies))
ImportError: Missing required dependencies ['pytz']

_hiwire_throw_error()@https://data-iodide.stage.mozaws.net/pyodide-0.2.0/pyodide.asm.js:8:39275
{anonymous}()@https://data-iodide.stage.mozaws.net/pyodide-0.2.0/pyodide.asm.wasm:wasm-function[359]:0xcf545
{anonymous}()@https://data-iodide.stage.mozaws.net/pyodide-0.2.0/pyodide.asm.wasm:wasm-function[368]:0xd0af8
pyodide/Module.__runPython()@https://data-iodide.stage.mozaws.net/pyodide-0.2.0/pyodide.asm.js:8:1947571
_runpython_init_js/Module._runPythonInternal()@https://data-iodide.stage.mozaws.net/pyodide-0.2.0/pyodide.asm.js:8:42686
internal()@https://data-iodide.stage.mozaws.net/pyodide-0.2.0/pyodide.asm.js:8:43256
runInternal()@https://data-iodide.stage.mozaws.net/pyodide-0.2.0/pyodide.asm.js:8:43628
```

So it appears that perhaps one of the dependencies fails due to a network issue in some way. Trying to reproduce w. the network panel open to see if that is indeed the issue.

Do we have a way to surface that type of error instead of `ImportError: Missing required dependencies ['pytz']`?

Commit message: Fix package loading race condition

Patch to review:
diff --git a/src/pyodide.js b/src/pyodide.js
index 56f5649a7fe..9f2189ca4cc 100644
--- a/src/pyodide.js
+++ b/src/pyodide.js
@@ -148,8 +148,11 @@ var languagePluginLoader = new Promise((resolve, reject) => {
         messageCallback(`Loading ${packageList}`);
       }
 
+      var packageCounter = Object.keys(toLoad).length * 2;
+
       window.pyodide._module.monitorRunDependencies = (n) => {
-        if (n === 0) {
+        packageCounter--;
+        if (packageCounter === 0) {
           for (let package in toLoad) {
             window.pyodide.loadedPackages[package] = toLoad[package];
           }


Code review comments:
diff_hunk: @@ -148,8 +148,11 @@ var languagePluginLoader = new Promise((resolve, reject) => {
         messageCallback(`Loading ${packageList}`);
       }
 
+      var packageCounter = Object.keys(toLoad).length * 2;
+
       window.pyodide._module.monitorRunDependencies = (n) => {
path: src/pyodide.js
text: @user1:
you don't need the `n` argument anymore here

@author:
Good point!