Ensure we've acquired the worker lock before passing it to the worker.

[pkaminski]

Otherwise, the worker might believe that the client is dead on arrival and destroy the connection immediately.

---

This change is 

[gemini-code-assist]

Summary of Changes

Hello @pkaminski, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical race condition in the worker connection process where a web worker could incorrectly perceive the client as unresponsive and terminate its connection prematurely. By introducing an explicit await for the worker liveness lock acquisition, it ensures proper synchronization. Additionally, it enhances the diagnostic information available when a worker terminates by including relevant shared state in the error object.

Highlights

• Worker Liveness Lock Synchronization: The connectWorker method now explicitly awaits the acquisition of the worker liveness lock before proceeding, preventing a race condition where the worker might prematurely terminate due to believing the client is unresponsive.
• Asynchronous Worker Connection: The connectWorker static method has been refactored to be async, allowing for proper await usage during the lock acquisition and bridge initialization process.
• Enhanced Error Reporting: Worker termination errors now include the _shared state in an extra field, providing more context for debugging issues related to worker crashes.
• Version Bump: The package version has been updated from 7.8.1 to 7.8.2.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly addresses a critical race condition by ensuring the client acquires the worker lock before initializing the bridge. The change to use async/await in connectWorker also improves code readability. The addition of extra context to crash reports is a good enhancement for debugging. I have one suggestion to make the new lock-waiting logic slightly more concise. The version bump is appropriate for this fix.

[gemini-code-assist]

While the current implementation correctly waits for the lock, the logic for creating and awaiting the lockReady promise can be simplified. You can wrap the navigator.locks.request call in a single new Promise and await that directly. This avoids the need for the separate resolveLockReady variable, making the code a bit more self-contained and easier to read.

Suggested change

	let resolveLockReady;
	const lockReady = new Promise(resolve => {resolveLockReady = resolve;});
	navigator.locks.request(webWorker.lockName, () => {
	resolveLockReady();
	// eslint-disable-next-line lodash/prefer-noop
	return new Promise(() => {/* release lock only on page exit or crash */});
	});
	await lockReady;
	await new Promise(resolve => {
	navigator.locks.request(webWorker.lockName, () => {
	resolve();
	// eslint-disable-next-line lodash/prefer-noop
	return new Promise(() => {/* release lock only on page exit or crash */});
	});
	});

[pkaminski]

@pkaminski resolved 1 discussion.
Reviewable status: 0 of 8 files reviewed, all discussions resolved (waiting on @snoack).

[snoack]

@snoack reviewed 8 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pkaminski).

---

src/Truss.js line 252 at r2 (raw file):

          resolve();
          // eslint-disable-next-line lodash/prefer-noop
          return new Promise(() => {/* release lock only on page exit or crash */});

I suppose we could use _.noop here though.

[pkaminski]

@pkaminski made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @snoack).

---

src/Truss.js line 252 at r2 (raw file):

Previously, snoack (Sebastian Noack) wrote…

I suppose we could use _.noop here though.

We could, but I liked the explicit comment on why we're using an unresolvable promise.

[pkaminski]

@pkaminski resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @pkaminski).