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

Fix: Playground not starting due to a race condition #1084

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Mar 5, 2024

Without this PR, the isConnected() method tries for two seconds before assuming the connection worked out. However, sometimes it takes more than that to reach the remote frame or context. This PR replaces that with an infinite loop that will keep trying until the remote end actually responds.

This solves a problem with the progress bar stuck at 0% or 50%.

Testing instructions

Apply the following patch to artificially and synchronously slow down the boot in all contexts (website, remote, worker)

diff --git a/packages/php-wasm/web/src/lib/web-php.ts b/packages/php-wasm/web/src/lib/web-php.ts
index 27b976a23..e4f9071b5 100644
--- a/packages/php-wasm/web/src/lib/web-php.ts
+++ b/packages/php-wasm/web/src/lib/web-php.ts
@@ -44,6 +44,10 @@ const fakeWebsocket = () => {
        };
 };

+for (let i = 0; i < 5000000; i++) {
+       crypto.randomUUID();
+}
+
 export class WebPHP extends BasePHP {
        /**
         * Creates a new PHP instance.

Then open the local Playground at http://localhost:5400/website-server/ and confirm it starts loading after a few seconds.

Repeat a dozen or so times and confirm Playground reliably loads each time.

Then do the same without this PR and confirm the progress bar tends to get stuck.

I'd love to ship an E2E test with this, but I can't think of a way to build one.

Without this PR, the isConnected() method tries for two seconds before
assuming the connection worked out. However, sometimes it takes more
than that to reach the remote frame or context. This PR replaces that
with an infinite loop that will keep trying until the remote end
actually responds.

This solves a problem with the progress bar stuck at 0% or 50%.

 ## Testing instructions

Apply the following patch to artificially and synchronously slow down the boot in all contexts (website, remote, worker)

```patch
diff --git a/packages/php-wasm/web/src/lib/web-php.ts b/packages/php-wasm/web/src/lib/web-php.ts
index 27b976a23..e4f9071b5 100644
--- a/packages/php-wasm/web/src/lib/web-php.ts
+++ b/packages/php-wasm/web/src/lib/web-php.ts
@@ -44,6 +44,10 @@ const fakeWebsocket = () => {
        };
 };

+for (let i = 0; i < 5000000; i++) {
+       crypto.randomUUID();
+}
+
 export class WebPHP extends BasePHP {
        /**
         * Creates a new PHP instance.
```

Then open the local Playground at http://localhost:5400/website-server/
and confirm it starts loading after a few seconds.

Repeat a dozen or so times and confirm Playground reliably loads each
time.

Then do the same without this PR and confirm the progress bar tends to
get stuck.

I'd love to ship an E2E test with this, but I can't think of a way to
build one.
@adamziel adamziel changed the title Fix: Playground not starting up due to a race condition Fix: Playground not starting due to a race condition Mar 5, 2024
@adamziel adamziel merged commit c0df8b5 into trunk Mar 5, 2024
5 checks passed
@adamziel adamziel deleted the fix-race-condition-on-startup branch March 5, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant