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

Crash when entering or previewing Home #356

Closed
Gajus84 opened this issue Nov 10, 2023 · 27 comments · Fixed by #357
Closed

Crash when entering or previewing Home #356

Gajus84 opened this issue Nov 10, 2023 · 27 comments · Fixed by #357
Labels
bug Something isn't working waiting on op Waiting for more information from the original poster
Milestone

Comments

@Gajus84
Copy link

Gajus84 commented Nov 10, 2023

What system are you running Yazi on?

Linux Wayland

What terminal are you running Yazi in?

wezterm-gui 20231107-082518-f0e3eecb

Yazi version

yazi 0.1.5

Did you try the latest main branch to see if the problem has already been fixed?

Tried, but the problem is still present

Describe the bug

Yazi crashes when entering or previewing home directory with following code:

thread 'tokio-runtime-worker' panicked at library/std/src/sys/unix/time.rs:77:9: assertion failed: tv_nsec >= 0 && tv_nsec

This does not happen further down the tree for example in .config/yazi

I was able to narrow it down to the feat: new reveal command #341 commit, if I'm not mistaken.

Expected Behavior

No crash in home directory.

To Reproduce

Not possible to say, without tracking down what file or else is causing the crash.

Configuration

happens also with no config.

Anything else?

Maybe you can give me further advise to track down the cause.

@Gajus84 Gajus84 added the bug Something isn't working label Nov 10, 2023
@sxyazi
Copy link
Owner

sxyazi commented Nov 10, 2023

Thanks for your report. After some investigation, it appears to be a Rust bug, rust-lang/rust#108277.

Your issue is similar to zellij-org/zellij#2369, and the root cause is the presence of files with invalid creation times in the $HOME. In such cases, Rust panics instead of returning an error.

The problem arose in this PR due to the inclusion of a performance optimization - parsing time and other meta information prematurely to avoid multiple subsequent parses.

I will attempt to add a catch_unwind as a "temporary" fix until Rust addresses it.

@sxyazi
Copy link
Owner

sxyazi commented Nov 10, 2023

Hi I made a PR to fix this, let me know if it works for you! #357

@sxyazi sxyazi added the waiting on op Waiting for more information from the original poster label Nov 10, 2023
@sxyazi sxyazi added this to the v0.1.6 milestone Nov 11, 2023
@Gajus84
Copy link
Author

Gajus84 commented Nov 11, 2023

Unfortunately still crashing.

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

Hi, could you try to apply the following three patches and see which one works? I can't reproduce it on my side since I don't have a file with invalid times.

- accessed: m.accessed().ok(),
+ accessed:  std::panic::catch_unwind(|| m.accessed().ok()).ok().flatten(),
- created:  m.created().ok(),
+ created:  std::panic::catch_unwind(|| m.created().ok()).ok().flatten(),
- modified: m.modified().ok(),
+ modified:  std::panic::catch_unwind(|| m.modified().ok()).ok().flatten(),

accessed: m.accessed().ok(),
created: m.created().ok(),
modified: m.modified().ok(),

@Gajus84
Copy link
Author

Gajus84 commented Nov 11, 2023

Hi, I tried every patch and then all three at once. Neither does solve the problem.

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

So weird, I checked and time operations only exist in this file. What would happen if you replaced these three with None?

- accessed: m.accessed().ok(),
+ accessed: None,
- created:  m.created().ok(),
+ created:  None,
- modified: m.modified().ok(),
+ modified: None,

@Gajus84
Copy link
Author

Gajus84 commented Nov 11, 2023

Yes that works. So it is the correct part of the code. Maybe the "catch_unwind" does not work?

I did test a little further and it is the "created" bit that causes the problem.

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

Ah yeah catch_unwind cannot catch all panics, but it is the only way we handle this issue.

I have pushed new code that rewrites Rust's default panic handler. Hope it will be effective. Please give it a try!

@Gajus84
Copy link
Author

Gajus84 commented Nov 11, 2023

Unfortunately still panics.

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

What's the output?

@Gajus84
Copy link
Author

Gajus84 commented Nov 11, 2023

Just the generic ZSH core dumped

zsh: IOT instruction (core dumped) yazi/target/release/yazi

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

use yazi_shared::Term;

pub(super) struct Panic;

impl Panic {
	pub(super) fn install() {
		better_panic::install();

		let hook = std::panic::take_hook();
		std::panic::set_hook(Box::new(move |info| {
			_ = Term::goodbye(|| {
				println!("{}", info.to_string());
				false
			});
		}));
	}
}

Please use this code to completely replace the yazi-fm/src/panic.rs file of that PR, and then paste the output here.

@Gajus84
Copy link
Author

Gajus84 commented Nov 11, 2023

panicked at library/std/src/sys/unix/time.rs:77:9: assertion failed: tv_nsec >= 0 && tv_nsec < NSEC_PER_SEC as i64

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

use yazi_shared::Term;

pub(super) struct Panic;

impl Panic {
	pub(super) fn install() {
		better_panic::install();

		let hook = std::panic::take_hook();
		std::panic::set_hook(Box::new(move |info| {}));
	}
}

Try this then

@Gajus84
Copy link
Author

Gajus84 commented Nov 11, 2023

Back to the generic core dumped.

Also it builds, but throws three warnings:

warning: unused import: "yazi_shared::Term"
warning: unused variable: "hook"
warning: unused variable: "info"

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

Have you restored catch_unwind for yazi/yazi-shared/src/cha.rs? It should be like this:

accessed: m.accessed().ok(), 
created: std::panic::catch_unwind(|| m.created().ok()).ok().flatten(),
modified: m.modified().ok(), 

@Gajus84
Copy link
Author

Gajus84 commented Nov 11, 2023

Yes, I altered and build it from an pr-64b2fdd0 checkout

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

Hmm no more idea, I'm not sure how to address it for now...

@Gajus84
Copy link
Author

Gajus84 commented Nov 11, 2023

I'm pretty sure that I can solve my problem by just moving/copy my files a little, so they get a new timestamp. Did not do that to allow testing.
But it might be an edge case and will not hit many until rust is fixing the problem.

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

I have decided to disable the creation time for now. This primarily affects users who sort files by creation time.

Disabling a sub-feature seems to have a smaller cost compared to the potential process crashes -- we are always to prevent the worst-case.

I will update the documentation to indicate that the feature is currently unavailable. Once Rust addresses it, we will restore the functionality.

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

I've pushed a new commit, please try it out to see if it's working now.

@Gajus84
Copy link
Author

Gajus84 commented Nov 11, 2023

It's working. I'll leave the files in my /home untouched as good as I can, to allow possible testing after a Rust-fix.
Pretty sure that some ancient dot-file is causing this.

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

Thank you very much for your assistance in testing, let's merge it now!

@Brixy
Copy link
Contributor

Brixy commented Nov 11, 2023

Thank you for this pull request.

Despite the merged pr yazi still panics.

Previewing the home directory is not (and never was) a problem. For me, yazi only becomes unresponsive with gh. Entering the home directory e.g. with vim keys works.

If there is anything I can test, please let me know.

@sxyazi
Copy link
Owner

sxyazi commented Nov 11, 2023

@Brixy Thank you for your report that helped me discover this potential bug.

I have created a quick fix for it, it should work. c49bf02

@Brixy
Copy link
Contributor

Brixy commented Nov 11, 2023

Everything works now, @sxyazi.

Thank you very much again for your superior maintenance!

Copy link

I'm going to lock this issue because it has been closed for 30 days. ⏳ This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working waiting on op Waiting for more information from the original poster
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants