multiaddr[1/N]: serialize static address creation

[hieblmi]

In order to remove the mutex in address/manager.go we serialize address creation through a channel in the Run loop.

[gemini-code-assist]

Summary of Changes

Hello @hieblmi, 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 refactors the staticaddr manager to remove a mutex by serializing static address creation through a channel. This change aims to improve the concurrency model for address management by centralizing the creation logic within the Run loop, making it more robust and easier to reason about.

Highlights

• Concurrency Model Refactor: Replaced sync.Mutex with a channel-based serialization mechanism for static address creation within the Manager.
• Channel-based Request Handling: Introduced request and response structs and a requests channel to handle address creation requests asynchronously and serially within the Manager's Run loop.
• API Adaptation: The public NewAddress method now sends requests to the internal Run loop via the new channel and waits for the result, abstracting the serialization logic.
• Internal Logic Encapsulation: The core logic for creating/retrieving static addresses was moved from NewAddress to a new private method newAddress, which no longer requires explicit locking.

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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

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 issue 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]

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

[hieblmi]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the address.Manager to serialize static address creation using a channel instead of a mutex. While the intent is good, the implementation has a critical flaw: it spawns a new goroutine for each address request, which defeats the serialization and reintroduces a race condition. My review includes a critical comment with a suggested fix to correctly serialize the operation, though it also points out the performance trade-offs of the simple fix.

[hieblmi]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the address manager to serialize static address creation using a channel and a worker goroutine, instead of a mutex. This is a good idiomatic change in Go that improves the concurrency model. The implementation is mostly correct, but I've found a high-severity issue where the worker goroutine is not panic-safe. A panic would crash the worker, making the address creation functionality unavailable until a restart. I've added a review comment with a suggestion to make the worker more robust by recovering from panics.

[hieblmi]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the static address manager to serialize address creation using a channel and a worker goroutine, removing the need for a mutex. The changes are well-implemented and follow standard Go concurrency patterns. The use of a buffered response channel and context handling in select statements makes the implementation robust against timeouts and cancellations. I have one suggestion to improve the structure and readability of the new addrWorker function.

[hieblmi]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the address creation logic in staticaddr/address/manager.go to use a channel for serialization instead of a mutex. This is a good architectural change that follows common Go concurrency patterns. The implementation is well-done, correctly handling context cancellations and potential panics in the worker goroutine. I have one suggestion to enhance panic logging for better debuggability.

[starius]

Why e, not err?

[hieblmi]

fixed

[starius]

Panic means a bug in the code. I think it's better to crash the process so this doesn't go unnoticed?

[hieblmi]

Yes I actually agree, I've removed the recovery here.