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

Heisenbug: trip _gateway panic sometimes #892

Closed
crrodriguez opened this issue Dec 31, 2023 · 8 comments · Fixed by #893
Closed

Heisenbug: trip _gateway panic sometimes #892

crrodriguez opened this issue Dec 31, 2023 · 8 comments · Fixed by #893
Labels
bug Something isn't working tui
Milestone

Comments

@crrodriguez
Copy link

#trip _gateway
thread 'main' panicked at /rustc/3cdd004e55c869faa2b7b25efd3becf50346e7d6/library/core/src/cmp.rs:879:9:
assertion failed: min <= max
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Unfortunately it cannot be reproduced the second time ..it happens once in a while.. I'l trry to get a backtrace ..

@fujiapple852
Copy link
Owner

fujiapple852 commented Dec 31, 2023

Thanks for reporting @crrodriguez.

please also try enabling verbose logging with -v which may help give more context around the failure.

To help narrow it down:

Does it happen immediately at startup or during the trace? Does it only happy with the Tui or can you reproduce with -m stream ?

Please also confirm the version of Trippy and your platform.

@fujiapple852
Copy link
Owner

fujiapple852 commented Dec 31, 2023

The assertion failure line from the reported error is from the clamp function:

https://github.com/rust-lang/rust/blob/3cdd004e55c869faa2b7b25efd3becf50346e7d6/library/core/src/cmp.rs#L879

The only place clamp is called (directly, at least) is here:

let count = hop.addr_count().clamp(1, max_addr as usize);

@fujiapple852
Copy link
Owner

@crrodriguez I can reproduce this failure by pressing } (default binding for expand-hosts-max), can you confirm this is what you are doing when it fails?

The bug here is in the TuiApp::max_hosts function:

/// The maximum number of hosts per hop for the currently selected trace.
pub fn max_hosts(&self) -> u8 {
self.selected_tracer_data
.hops(self.selected_flow)
.iter()
.map(|h| h.addrs().count())
.max()
.and_then(|i| u8::try_from(i).ok())
.unwrap_or_default()
}
}

If there are no hops reported or none of the reported hops contain any addresses, then the function returns 0 (i.e. u8::default() from unwrap_or_default()). This will cause TuiApp::expand_hosts_max to set max_addrs as Some(0):

pub fn expand_hosts_max(&mut self) {
self.tui_config.max_addrs = Some(self.max_hosts());
}

With that in place, we will now get a call to clamp(1, 0) which will lead to the assertion failure observed:

let count = hop.addr_count().clamp(1, max_addr as usize);

The fix is to ensure that in such cases max_addrs is set to be None.

Given you are running trip _gateway i'm assuming that means _gateway is the first (and only) hop and so it would seem likely that if the _gateway hop does not respond to the IMCP probe then we would indeed end up in a situation where none of the hops contain any addresses and would therefore lead to this bug.

@fujiapple852 fujiapple852 added the bug Something isn't working label Dec 31, 2023
@fujiapple852 fujiapple852 self-assigned this Dec 31, 2023
@fujiapple852 fujiapple852 added this to the 0.10.0 milestone Dec 31, 2023
fujiapple852 added a commit that referenced this issue Dec 31, 2023
…op (#892)

The `expand-hosts-max` (default binding: `}`) TUI command panics if there
are no addresses for any hop.
@fujiapple852
Copy link
Owner

Fixed in #893 which will go into the 0.10.0 release.

@crrodriguez
Copy link
Author

crrodriguez commented Dec 31, 2023

@crrodriguez I can reproduce this failure by pressing } (default binding for expand-hosts-max), can you confirm this is what you are doing when it fails?

Yes, See in my spanish language keyboard that key is annoyingly too close to enter and sometimes both get pressed..

I updated to https://github.com/fujiapple852/trippy.git#58dc9b58) and the problem still there. I 'll try your fix.
Thanks.

@c-git
Copy link
Collaborator

c-git commented Dec 31, 2023

Thank you very much for testing and confirming that works @crrodriguez

@c-git c-git closed this as completed Dec 31, 2023
fujiapple852 added a commit that referenced this issue Jan 1, 2024
…op (#892)

The `expand-hosts-max` (default binding: `}`) TUI command panics if there
are no addresses for any hop.
@fujiapple852
Copy link
Owner

I merged #893 so you can retry from the latest master (f07d58f) if you wish.

@crrodriguez
Copy link
Author

I merged #893 so you can retry from the latest master (f07d58f) if you wish.

, I did try it and the issue is gone. thanks.!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tui
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants