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

Unexpected Crash when performing a reopen of a "Recent Project" #6835

Closed
1 task done
wintermi opened this issue Jan 27, 2024 · 11 comments
Closed
1 task done

Unexpected Crash when performing a reopen of a "Recent Project" #6835

wintermi opened this issue Jan 27, 2024 · 11 comments
Labels
defect [core label] panic / crash [core label]

Comments

@wintermi
Copy link

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

I requested to open a "Recent Project" as shown in the below screenshot.

image

Zed crashed at this point

Environment

Zed: v0.119.20 (Zed)
OS: macOS 14.3.0
Memory: 16 GiB
Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

Apple Crash Report:

Apple Crash Report.log

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

If you only need the most recent lines, you can run the zed: open log command palette action to see the last 1000.

No response

@wintermi wintermi added admin read Pending admin review defect [core label] triage Maintainer needs to classify the issue labels Jan 27, 2024
@JosephTLyons JosephTLyons added panic / crash [core label] and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Jan 29, 2024
@JosephTLyons
Copy link
Contributor

Thanks for the crash report @wintermi

@ConradIrwin
Copy link
Member

This looks like a crash we've been seeing with alacrity, the panic is on a background thread (number 46) running:

  • std::sys_common::backtrace::__rust_begin_short_backtrace::<<alacritty_terminal::event_loop::EventLoop<alacritty_terminal::tty::unix::Pty, terminal::ZedListener>>::spawn::{closure#0}, (alacritty_terminal::event_loop::EventLoop<alacritty_terminal::tty::unix::Pty, terminal::ZedListener>, alacritty_terminal::event_loop::State)>

cc @mrnugget

@mrnugget
Copy link
Member

Hey @wintermi! Thanks for reporting — familiar avatar, from the Ghostty discord :) I'll try to look into this tomorrow, since I can now correlate this issue with the crash report we saw. Should probably help me reproduce it.

@mrnugget
Copy link
Member

@wintermi can you reliably reproduce this? If so, what are the steps to reproduce this? For the life of me, I can't figure out why terminal would crash. Maybe there's something specific to your terminal setup?

I'm happy to hop on a call, btw.! I vaguely remember that we're in the same timezone.

@mrnugget
Copy link
Member

A few more follow-up questions after digging into this a bit more:

  1. Just to confirm: it happens when you select the project to be opened, right? It doesn't happen when you see the dialog box to select a project?
  2. Does this only happen when you open Ghostty?
  3. How big is the terminal window before you open the recent project?
  4. Does this happen when you comment-out your whole shell-rc file? I'm wondering whether something in how your shell starts causes the allocated PTY to die which then causes the panic here.

On (4) and leaving some breadcrums:

I had previously assumed that between this call here

//Setup the pty...
let pty = match tty::new(
&pty_options,
TerminalSize::default().into(),
window.window_id().as_u64(),
) {

which allocates a new PTY with openpty and this call where the crash happens

let _io_thread = event_loop.spawn(); // DANGER

Nothing really happens except the openpty call. But now reading this again, I realise: quite a lot might happen! Because tty:new spawns the shell:

https://github.com/alacritty/alacritty/blob/7c046f5ae664f746c5f5a4bbe18c0e89f5702305/alacritty_terminal/src/tty/unix.rs#L197-L296

So my current theory is that: openpty works and succeeds, but then something goes wrong that makes the allocated master_fd of the PTY invalid, which then leads to .spawn crashing. Now we just need to figure out what goes wrong.

@wintermi
Copy link
Author

@wintermi can you reliably reproduce this? If so, what are the steps to reproduce this? For the life of me, I can't figure out why terminal would crash. Maybe there's something specific to your terminal setup?

I'm happy to hop on a call, btw.! I vaguely remember that we're in the same timezone.

I have not found a way to reliably reproduce the crash, in fact so far I have only experienced it 2 out 40 attempts. Both crashed after opening a Recent Project, and both crashes happened unexpectedly, while not trying to reproduce the issue. All attempts trying to reproduce the crash from a fresh start failed.

@mrnugget
Copy link
Member

Both crashed after opening a Recent Project

Always the same project? Or a different one?

@wintermi
Copy link
Author

Both crashed after opening a Recent Project

Always the same project? Or a different one?

For the first crash, I was switching between 2 projects and the crash occurred after the selection of the Ghostty project. For the second crash, I was switching between 3 project and the crash did occur again on the selection of the Ghostty project.

I have however not been able to reproduce the issue since upgrading to the latest version Zed 0.119.21.

@wintermi
Copy link
Author

A few more follow-up questions after digging into this a bit more:

  1. Just to confirm: it happens when you select the project to be opened, right? It doesn't happen when you see the dialog box to select a project?

After selecting the project.

  1. Does this only happen when you open Ghostty?

So far, yes.

  1. How big is the terminal window before you open the recent project?

I never had the terminal window open on both crashes, or prior to the crash. I generally Cmd + Tab to use Ghostty.

  1. Does this happen when you comment-out your whole shell-rc file? I'm wondering whether something in how your shell starts causes the allocated PTY to die which then causes the panic here.

As I have not opened the terminal window, I assume that the shell is never started, or does Zed always start the shell?

@mrnugget
Copy link
Member

I never had the terminal window open on both crashes, or prior to the crash. I generally Cmd + Tab to use Ghostty.
As I have not opened the terminal window, I assume that the shell is never started, or does Zed always start the shell?

Hmmm, interesting. Zed remembers whether you had a terminal open for a given project. So if you open Ghostty in Zed, open a terminal, then close the window, then use Open recent project and select Ghostty, it should spawn with the terminal window open.

The code that caused your panic originates from the terminal-spawn code, which only gets executed when a terminal is opened. So my guess would be that when you last had the Ghostty project open, there was a terminal open, and when opening the project we restored it, which then leads to the crash.

ConradIrwin added a commit that referenced this issue Feb 3, 2024
ConradIrwin added a commit that referenced this issue Feb 3, 2024
Release Notes:

- Fixed some panics in the Terminal
([#6835](#6835)).
ConradIrwin added a commit that referenced this issue Feb 8, 2024
Release Notes:

- Fixed some panics in the Terminal
([#6835](#6835)).
@ConradIrwin
Copy link
Member

This was fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect [core label] panic / crash [core label]
Projects
None yet
Development

No branches or pull requests

4 participants