Skip to content

fix(upnp): panic during a shutdown process#5998

Merged
mergify[bot] merged 3 commits intolibp2p:masterfrom
ackintosh:fix-upnp
Apr 23, 2025
Merged

fix(upnp): panic during a shutdown process#5998
mergify[bot] merged 3 commits intolibp2p:masterfrom
ackintosh:fix-upnp

Conversation

@ackintosh
Copy link
Contributor

@ackintosh ackintosh commented Apr 19, 2025

Description

upnp panics here in case where the gateway state is Searching and the task executor is shutting down, since the search_gateway task is terminated and the sender channel has been dropped.

Here is an example code that reproduces the issue based on the upnp example, along with the corresponding error message.

Since this case is expected during shutdown, I believe upnp should avoid panicking and continue its operation.

Notes & open questions

Should we add new variant to GatewayState and Event for this?

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@ackintosh ackintosh marked this pull request as ready for review April 20, 2025 22:30
jxs
jxs previously approved these changes Apr 22, 2025
Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this Akihito! LGTM! ❤️

@jxs jxs added the send-it label Apr 22, 2025
@mergify mergify bot dismissed jxs’s stale review April 22, 2025 18:39

Approvals have been dismissed because the PR was updated after the send-it label was applied.

@mergify mergify bot merged commit 9ca0d44 into libp2p:master Apr 23, 2025
71 checks passed
@ackintosh ackintosh deleted the fix-upnp branch April 26, 2025 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants