repro: Add test where Promise.all checks if they can run in parallel

[grabbou]

On Android, when there are two promises, second promise will wait for the first one to resolve.

In this example, the resolution should be:

• Promise.all(foo(), bar()) called, both functions are running
• foo() is pending
• bar() is pending
• bar gets resolved
• 2 seconds later
• foo() gets resolved
• Promise.all gets resolved

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

| Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **nitro-docs** | ⬜️ Skipped ([Inspect](https://vercel.com/margelo/nitro-docs/Ee96UMn51TaHLNxKvkmrjHm5J9Z3)) | | | Nov 18, 2024 11:25pm |

[mrousavy]

Hm, the .Default scope should allow for parallelism - it is a shared pool. https://github.com/mrousavy/nitro/blob/d76d0c02621d4183544d2a8af5b8b334318b4551/packages/react-native-nitro-modules/android/src/main/java/com/margelo/nitro/core/Promise.kt#L57

I'd have to test this more, but in the meantime you can play around with this to see if changing to a different Dispatcher changes things? E.g. a Executors.newFixedThreadPool(4).asCoroutineDispatcher().

[mrousavy]

Btw.; didn't you talk about a different issue on WhatsApp? You mentioned that resolve just doesn't work anymore, and that it starts to hang in JSPromise, but it worked in Promise.kt and JPromise.hpp...?

[grabbou]

Btw.; didn't you talk about a different issue on WhatsApp? You mentioned that resolve just doesn't work anymore, and that it starts to hang in JSPromise, but it worked in Promise.kt and JPromise.hpp...?

Yes, it's still same issue, and this test shows its symptoms! Something stops JS from resolving the promise and my debugger breakpoints reveal that it's not hitting the JSPromise code

[grabbou]

The current test is invalid - we cannot test order of that. It's how native threads work, they can race.

We can test however if both promises resolve one after another - as that was the original issue you sent me on WhatsApp

Hm, I am not sure if that was the original issue and whether your updated test does show the real problem.

The issue is not promises running in parallel.

The issue is if there are two promises that run in parallel, one is wait 4, second one is wait 2, the second one will not resolve until first one resolved, so eventually they both resolve after 4 seconds

In practice, second one should resolve earlier, and code that follows after its await should be executed while we're waiting for first promise.

In my code, those promises are run from two different places (not Promise.all) but I did that just to demonstrate the case.

I guess I'll have to send a PR with a different test case to make this explicit

[grabbou]

Okay, I run this test on main and issue is gone. this passes. Promises resolve after the right time (4 and 1s respectively). Before, it would be always 4. Potentially related https://github.com/mrousavy/nitro/pull/347

 createTest('twoPromises can run in parallel', async () =>
      (
        await it(async () => {
          const foo = async () => {
            const start = performance.now()
            await testObject.wait(4)
            const end = performance.now()
            return end - start < 4 * 1000 + 10
          }
          const bar = async () => {
            const start = performance.now()
            await testObject.wait(1)
            const end = performance.now()
            return Math.round(end - start) < 1 * 1000 + 10
          }
          const [fooTime, barTime] = await Promise.all([foo(), bar()])
          return [fooTime, barTime]
        })
      )
        .didNotThrow()
        .equals([true, true])
    ),

[mrousavy]

Yea I might've fixed this in #347 🙈