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

Catching panics #1389

Open
Stebalien opened this issue Apr 11, 2022 · 1 comment
Open

Catching panics #1389

Stebalien opened this issue Apr 11, 2022 · 1 comment
Assignees
Labels
P3 Low: Not priority right now

Comments

@Stebalien
Copy link
Member

Stebalien commented Apr 11, 2022

(moving a discussion from a private conversation to somewhere more public)

Libp2p performs quite a bit of complex parsing, which has occasionally lead to panics at runtime. When uncaught, these panics crash the entire node.

Proposal: Catch panics at "failure boundaries". E.g.:

  • If we have some form of "connection" worker, catch panics in the worker and kill the entire connection if the worker panics. Same for streams.
  • Catch panics in per-peer stream handlers, cleaning up all state related to the peer.
  • Catch panics in low-level parsing logic. Parsing tends to be pretty self-contained but also pretty error prone.
  • Stretch: Where possible, catch service-level panics, cancel all current requests, close all resources, and restart. But we do need to be a bit careful to not continue running in a corrupted state.
Stebalien added a commit to libp2p/go-libp2p-core that referenced this issue Apr 14, 2022
Part of libp2p/go-libp2p#1389

These kinds of functions:

1. Handle user input.
2. Often have out-of-bounds, null pointer, etc bugs.
3. Have completely isolated logic where local panics are unlikely to
   cause memory corruption elsewhere.
Stebalien added a commit to libp2p/go-yamux that referenced this issue Apr 14, 2022
Stebalien added a commit to libp2p/go-yamux that referenced this issue Apr 14, 2022
Stebalien added a commit to libp2p/go-yamux that referenced this issue Apr 14, 2022
Stebalien added a commit to libp2p/go-libp2p-core that referenced this issue Apr 16, 2022
Part of libp2p/go-libp2p#1389

These kinds of functions:

1. Handle user input.
2. Often have out-of-bounds, null pointer, etc bugs.
3. Have completely isolated logic where local panics are unlikely to
   cause memory corruption elsewhere.
Stebalien added a commit to libp2p/go-libp2p-core that referenced this issue Apr 18, 2022
Part of libp2p/go-libp2p#1389

These kinds of functions:

1. Handle user input.
2. Often have out-of-bounds, null pointer, etc bugs.
3. Have completely isolated logic where local panics are unlikely to
   cause memory corruption elsewhere.
marten-seemann pushed a commit to libp2p/go-libp2p-core that referenced this issue Apr 18, 2022
* feat: harden encoding/decoding functions against panics

Part of libp2p/go-libp2p#1389

These kinds of functions:

1. Handle user input.
2. Often have out-of-bounds, null pointer, etc bugs.
3. Have completely isolated logic where local panics are unlikely to
   cause memory corruption elsewhere.

* test: add a panic catcher test
Stebalien added a commit to libp2p/go-libp2p-tls that referenced this issue Apr 19, 2022
marten-seemann pushed a commit that referenced this issue Apr 27, 2022
marten-seemann pushed a commit that referenced this issue Aug 17, 2022
* feat: harden encoding/decoding functions against panics

Part of #1389

These kinds of functions:

1. Handle user input.
2. Often have out-of-bounds, null pointer, etc bugs.
3. Have completely isolated logic where local panics are unlikely to
   cause memory corruption elsewhere.

* test: add a panic catcher test
@BigLep BigLep added this to the Best Effort Track milestone Sep 9, 2022
@BigLep BigLep added P2 Medium: Good to have, but can wait until someone steps up P3 Low: Not priority right now and removed P2 Medium: Good to have, but can wait until someone steps up labels Sep 9, 2022
@BigLep BigLep linked a pull request Sep 16, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Low: Not priority right now
Projects
Status: 🥞 Todo
Development

Successfully merging a pull request may close this issue.

3 participants