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

always crashes on resize #1551

Closed
Tracked by #1100
dmd opened this issue Jun 30, 2022 · 21 comments · Fixed by #1552
Closed
Tracked by #1100

always crashes on resize #1551

dmd opened this issue Jun 30, 2022 · 21 comments · Fixed by #1552
Labels
stability Issues in relation to stability suspected bug

Comments

@dmd
Copy link

dmd commented Jun 30, 2022

Is zellij supposed to do something reasonable if you resize the terminal? I find that it always, 100% of the time, crashes if you resize the terminal.

Error occurred in server:

  × Thread 'screen' panicked.
  ├─▶ Originating Thread(s)
  │   	1. stdin_handler_thread: AcceptInput
  │   	2. screen_thread: TerminalPixelDimensions
  │
  ├─▶ At zellij-server/src/screen.rs:490:46
  ╰─▶ attempt to divide by zero
  help: If you are seeing this message, it means that something went wrong.
        Please report this error to the github issue.
        (https://github.com/zellij-org/zellij/issues)

        Also, if you want to see the backtrace, you can set the `RUST_BACKTRACE` environment variable to `1`.

Basic information

zellij --version:0.29.1
uname -av or ver(Windows): Linux micc 3.10.0-693.el7.x86_64 #1 SMP Thu Jul 6 19:56:57 EDT 2017 x86_64 x86_64 x86_64 GNU/Linux

@raphCode raphCode added the stability Issues in relation to stability label Jun 30, 2022
@raphCode
Copy link
Contributor

raphCode commented Jun 30, 2022

Uh, that is pretty bad and should not happen. Thanks for reporting!

What terminal emulator are you using?

Crash location:

let character_cell_size_height = text_area_size.height / self.size.rows;

Either we are doing something strange or the terminal reports a size of 0 briefly? Don't know, just reads like it.

@dmd
Copy link
Author

dmd commented Jun 30, 2022

Latest iTerm2 on Mac.

@imsnif
Copy link
Member

imsnif commented Jun 30, 2022

I think we might have fixed this in 0.30.0 - could you try upgrading?

@dmd
Copy link
Author

dmd commented Jun 30, 2022

Sorry, same behavior in 0.30.0.

Here is an interesting clue:

  1. start zellij
  2. resize window
  3. It crashes
  4. start zellij
  5. IMMEDIATE crash, without resizing

I have to logout/in of that ssh session before zellij works again.

@raphCode
Copy link
Contributor

raphCode commented Jun 30, 2022

After the first crash, can you run tput cols; tput lines?
I suspect these are reset to zero size.
You coud try to run eval $(resize) to set the terminal size correctly again.

I can reproduce the same error message by running stty rows 0 cols 0 before starting zellij.

@dmd
Copy link
Author

dmd commented Jun 30, 2022

Nope, they're not zero: https://shot.3e.org/ss-20220630_173722.mp4

@raphCode
Copy link
Contributor

Mhh, even with my own reproducer with stty rows 0 cols 0 tput reports non-zero values afterwards even though zellij crashes.
Not sure where tput gets its data from, it seems to make a TIOCGWINSZ ioctl like zellij.

You could try something like stty rows 20 cols 20 after a crash, maybe zellij works again after this command?

@dmd
Copy link
Author

dmd commented Jun 30, 2022

Yes, zellij works after that command.

@imsnif
Copy link
Member

imsnif commented Jul 1, 2022

Zellij seems to be getting a bad terminal window size report from the machine for some reason. We should handle it more gracefully, for sure.

@dmd - could oy help us out by testing some cases?

Could you issue tput cols; tput lines when:

  1. Logging into the remote machine with ssh before starting Zellij
  2. Logging into the remote machine with ssh, resizing the terminal window before starting Zellij
  3. Immediately after the crash
  4. After the crash and after resizing the terminal window

Thanks!

@raphCode
Copy link
Contributor

raphCode commented Jul 1, 2022

I am still pretty sure a window size of zero is reported, just tput is doing more than we asking for: It queries the terminal database and reports a default size of 80x24 if the size read is zero. (Thanks @tlinford for noticing) This is also what happens in your screencast.

A better command is stty size. Just for the fun, you can try running it after a resize/crash and compare the output to tput.

Also please try the resize command (likely part of the xterm package) to see if it can recover the correct size after a window resize. If yes, we can implement its behavior and make zellij work for your case.

@dmd
Copy link
Author

dmd commented Jul 1, 2022

Wow, ok. This turned out to be the result of a real weird very-special-to-my-situation setup that I'm sure doesn't apply to anyone else, so I think you don't need to worry about it at all.

The long story is I forgot that my 'm' alias which connects to my server actually doesn't just call ssh directly, it calls an Expect script similar to this one which, in my case, basically says "are you on the VPN? if so, connect this way ... if not, connect that way."

This expect script has a section in it that looked like this:

trap {
    set rows [system tput lines]
    set cols [system tput cols]
    stty rows $rows cols $cols < $spawn_out(slave,name)
} WINCH

This has worked just fine for most purposes for years. Oddly, I have no idea how this could have ever worked, because rows and cols are actually getting set to the empty string! Yet, somehow, the results of tput end up correct if run after this happens -- and tmux allows resizing correctly -- and if I comment out any subset of that trap section, it does not.

But if I change [system tput lines] to [stty rows] and [system tput cols] to [stty columns], everything (both tmux and zellij) works just fine, and the issue with zellij goes away entirely.

So unless you want to dig deeper into this morass of weirdness, I think we're done here.

@imsnif
Copy link
Member

imsnif commented Jul 1, 2022

Quite peculiar. I'm very curious how tmux works with this. Does it react properly to window changes? As in, expands or shrink to the window size?

@dmd
Copy link
Author

dmd commented Jul 1, 2022

Yes, tmux responded properly even on my old possibly-broken version of the script.

@imsnif
Copy link
Member

imsnif commented Jul 1, 2022

@raphCode - any guesses? Would be cool to gracefully just solve this problem when it happens.

@tlinford
Copy link
Contributor

tlinford commented Jul 1, 2022

@imsnif
Copy link
Member

imsnif commented Jul 1, 2022

Hum - nice... but then, how does the UI know the expand/shrink to the correct size?

@tlinford
Copy link
Contributor

tlinford commented Jul 1, 2022

running tmux after doing stty rows 0 cols 0 looks like this:
screenshot-2022-07-01T14:28:48

@imsnif
Copy link
Member

imsnif commented Jul 1, 2022

But for @dmd it does resize in this state

@tlinford
Copy link
Contributor

tlinford commented Jul 1, 2022

no clue! I was more thinking of wether we should handle the case of rows/cols = 0 in a similar way by falling back to defaults, to avoid crashes?

@imsnif
Copy link
Member

imsnif commented Jul 1, 2022

For sure! I just would also like us to be able to handle this as gracefully as tmux by just continuing to work :)

@raphCode
Copy link
Contributor

raphCode commented Jul 3, 2022

Oddly, I have no idea how this could have ever worked, because rows and cols are actually getting set to the empty string!

On my end, stty 9.1 does not even accept this:

$ stty rows '' cols ''
stty: invalid integer argument: ‘’

@raphCode - any guesses?

Nothing really conclusive, but some thoughts on how it is possible to recover the correct terminal size despite the tty set to zero columns / rows:

There are sometimes window pixel sizes reported in addition to the terminal size in characters:
ioctl(0, TIOCGWINSZ, {ws_row=19, ws_col=78, ws_xpixel=0, ws_ypixel=0}) = 0
Here they are zero but I definitely saw non-zero values there on another machine. By diving by the pixel per character value the chracter size can be calculated.

The resize command in the man page describes that it uses a combination of tty ioctls ("stty syscalls") and "asking the terminal for size" using escape codes. Those CSI escape codes are answered by the terminal returning the size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stability Issues in relation to stability suspected bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants