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

Exception when switching buffer after resize #842

Closed
mofux opened this issue Aug 5, 2017 · 11 comments
Closed

Exception when switching buffer after resize #842

mofux opened this issue Aug 5, 2017 · 11 comments
Assignees
Labels
type/bug Something is misbehaving
Milestone

Comments

@mofux
Copy link
Contributor

mofux commented Aug 5, 2017

Problem

An exception is thrown (latest master and probably before) if we resize the screen to a bigger dimension while being in the alt screen buffer. Once the alt screen buffer exists and we switch back to the normal buffer, the exception will show up, and the terminal will stop working (blank screen).

I think this has the same root cause as #838 and likely others where the line or row data is an undefined reference.

screen shot 2017-08-06 at 01 31 28

How to reproduce

  • Start the demo
  • Run a command that uses the alt screen buffer (e.g. vim)
  • Change the rows and/or cols value to a higher value (it happens in both cases)
  • Exit the app to switch back to the normal buffer

Demo

crash

@mofux
Copy link
Contributor Author

mofux commented Aug 6, 2017

Obviously this happens because the renderer assumes that the dimensions of the normal buffer is in sync with the rows / cols, but it isn't. I think we have to adjust the size of a buffer to the current dimensions when activating it -- and before the renderer touches it.

@Tyriar Tyriar added the type/bug Something is misbehaving label Aug 6, 2017
@Tyriar Tyriar added this to the 2.9.1 milestone Aug 6, 2017
@Tyriar
Copy link
Member

Tyriar commented Aug 6, 2017

Can reproduce with vim and 40x40

@Tyriar
Copy link
Member

Tyriar commented Aug 6, 2017

It appears to only happen when the normal buffer isn't filled.

@Tyriar
Copy link
Member

Tyriar commented Aug 6, 2017

Yeah it's related to not filling in the normal buffer's rows when the rows increase. Can reproduce with vim and rows+1.

@Tyriar
Copy link
Member

Tyriar commented Aug 6, 2017

Related to #510. I think when BufferSet was added, checks were removed so we no longer clear the normal buffer when resizing in the alt buffer. Apparently we missed this one case.

@Tyriar
Copy link
Member

Tyriar commented Aug 6, 2017

@Tyriar
Copy link
Member

Tyriar commented Aug 6, 2017

The same also breaks when rows change in the other direction, the scrollbar shows there is more content but you can't scroll it (because they're blank lines).

@Tyriar Tyriar self-assigned this Aug 6, 2017
@Tyriar
Copy link
Member

Tyriar commented Aug 6, 2017

Obscure bug I ran into while coming up for a fix here. after activateAltBuffer if called, there is a hard reset on the terminal which tries to carry buffers over but it's initialized and buffer (the convenience var) is set to the active buffer that we create and discard.

Tyriar added a commit to Tyriar/xterm.js that referenced this issue Aug 6, 2017
This brings in proper support to resize both buffers (xtermjs#510) and fixes an
exception that was caused by wrongfully not clearing the normal buffer when
resizing while the alt buffer is active.

There was an obscure bug in this that could have caused some great confusion
later on; When switching to the alt buffer, a hard terminal reset was performed
which tried to retain the buffers. However, because buffers was initialized in
the Terminal constructor to a new BufferSet, the Terminal.buffer convenience
pointer was pointing at a stale alt buffer which was the one actually being
used, not Terminal.buffers.alt.

Fixes xtermjs#842
Fixes xtermjs#510
@mofux
Copy link
Contributor Author

mofux commented Aug 6, 2017

You're a genius, thanks @Tyriar!

@ZoeyR
Copy link
Contributor

ZoeyR commented Aug 8, 2017

Hey, I just thought I'd report that I'm still having this issue with the 2.9.1 release of xtermjs. Right after I call term.open(...) I call term.fit() and I get the same exception. It's not 100% consistent but it happens frequently enough to be an issue. 2.8.1 does not have this problem for me.

@parisk
Copy link
Contributor

parisk commented Aug 8, 2017

@dgriffen is there any chance that what you are reporting is this: #860?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is misbehaving
Projects
None yet
Development

No branches or pull requests

4 participants