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

(paged-terminal) Various panics/asserts #1591

Closed
3 tasks done
Tracked by #1584
qwerasd205 opened this issue Mar 17, 2024 · 10 comments
Closed
3 tasks done
Tracked by #1584

(paged-terminal) Various panics/asserts #1591

qwerasd205 opened this issue Mar 17, 2024 · 10 comments

Comments

@qwerasd205
Copy link
Collaborator

qwerasd205 commented Mar 17, 2024

I'm just gonna list a collection of different panics/asserts that I've run in to on the paged-terminal branch.

Fixed

  • Weird Scroll scrollUp
  • Weird Scroll scrollDown
  • Big Pager

Weird Scroll

Repro

  • Apply some change like fix(terminal/stream): correct invalid assertion #1590 to avoid triggering that assertion by accident.
  • cat /dev/urandom
  • Wait a bit, might take up to a minute or so. (A larger terminal seems to help)
  • Unfortunately, this seems to be stateful, it can't seemingly be triggered by a single control sequence on a fresh surface.

Traces

scrollUp:

/Users/qwerasd/Documents/ghostty/src/terminal/page.zig:290:19: 0x1047fb0b7 in getCells (ghostty)
            assert(@intFromPtr(row) < @intFromPtr(cells));
                  ^
/Users/qwerasd/Documents/ghostty/src/terminal/Terminal.zig:1399:36: 0x104932dbb in deleteLines (ghostty)
        const cells = page.getCells(row);
                                   ^
/Users/qwerasd/Documents/ghostty/src/terminal/Terminal.zig:1196:21: 0x1048cd0f7 in scrollUp (ghostty)
    self.deleteLines(count);
                    ^
/Users/qwerasd/Documents/ghostty/src/termio/Exec.zig:2449:31: 0x104a47b07 in scrollUp (ghostty)
        self.terminal.scrollUp(count);
                              ^
/Users/qwerasd/Documents/ghostty/src/terminal/stream.zig:566:80: 0x104a537b3 in csiDispatch (ghostty)
                    0 => if (@hasDecl(T, "scrollUp")) try self.handler.scrollUp(
                                                                               ^
/Users/qwerasd/Documents/ghostty/src/terminal/stream.zig:279:71: 0x104a672f7 in nextNonUtf8 (ghostty)
                    .csi_dispatch => |csi_action| try self.csiDispatch(csi_action),
                                                                      ^

scrollDown:

/Users/qwerasd/Documents/ghostty/src/terminal/size.zig:39:19: 0x1068f37e7 in ptr__anon_215959 (ghostty)
            assert(addr % @alignOf(T) == 0);
                  ^
/Users/qwerasd/Documents/ghostty/src/terminal/page.zig:293:36: 0x10698b0db in getCells (ghostty)
        const cells = row.cells.ptr(self.memory);
                                   ^
/Users/qwerasd/Documents/ghostty/src/terminal/Terminal.zig:1305:36: 0x106bd72c3 in insertLines (ghostty)
        const cells = page.getCells(row);
                                   ^
/Users/qwerasd/Documents/ghostty/src/terminal/Terminal.zig:1174:21: 0x106bd7c47 in scrollDown (ghostty)
    self.insertLines(count);
                    ^
/Users/qwerasd/Documents/ghostty/src/termio/Exec.zig:2445:33: 0x106bd7b7b in scrollDown (ghostty)
        self.terminal.scrollDown(count);
                                ^
/Users/qwerasd/Documents/ghostty/src/terminal/stream.zig:584:82: 0x106be38b3 in csiDispatch (ghostty)
                'T' => if (@hasDecl(T, "scrollDown")) try self.handler.scrollDown(
                                                                                 ^
/Users/qwerasd/Documents/ghostty/src/terminal/stream.zig:279:71: 0x106bf72f7 in nextNonUtf8 (ghostty)
                    .csi_dispatch => |csi_action| try self.csiDispatch(csi_action),
                                                                      ^

Big Pager

Repro

  • Make a fairly large terminal, e.g. by making the font small and making the term full screen.
  • git log
  • Scroll down a bit.
  • q
  • git log
  • Scroll down 1 row and up 1 row.
  • This should trigger the crash - if it doesn't, repeat these actions a couple times, and try scrolling down even further.

Trace

/Users/qwerasd/Documents/ghostty/src/terminal/size.zig:39:19: 0x103d737e7 in ptr__anon_215959 (ghostty)
            assert(addr % @alignOf(T) == 0);
                  ^
/Users/qwerasd/Documents/ghostty/src/terminal/page.zig:250:50: 0x103e60f3f in cloneFrom (ghostty)
            const other_cells = src_row.cells.ptr(other.memory)[0..cell_len];
                                                 ^
/Users/qwerasd/Documents/ghostty/src/terminal/PageList.zig:357:32: 0x103f8219f in clone (ghostty)
        try page.data.cloneFrom(
                               ^
/Users/qwerasd/Documents/ghostty/src/terminal/Screen.zig:226:37: 0x103f8412f in clonePool (ghostty)
    var pages = try self.pages.clone(.{
                                    ^
/Users/qwerasd/Documents/ghostty/src/terminal/Screen.zig:208:30: 0x103f84e03 in clone (ghostty)
    return try self.clonePool(alloc, null, top, bot);
                             ^
/Users/qwerasd/Documents/ghostty/src/renderer/Metal.zig:659:58: 0x103fb1bf7 in updateFrame (ghostty)
        var screen_copy = try state.terminal.screen.clone(
                                                         ^
/Users/qwerasd/Documents/ghostty/src/renderer/Thread.zig:463:27: 0x1040198c3 in callback (ghostty)
    t.renderer.updateFrame(
                          ^
@rockorager
Copy link
Collaborator

rockorager commented Mar 17, 2024

EDIT by mitchellh: this is resolved: 4fe49c7

Here's the one I get when moving tiled windows around (probably due to the resize I assume)

/home/tim/.zig/versions/zig-linux-x86_64-0.12.0-dev.3336+dbb11915b/lib/std/debug.zig:403:14: 0x1a6300c in assert (ghostty)
    if (!ok) unreachable; // assertion failure
             ^
/home/tim/repos/ghostty/src/terminal/page.zig:302:15: 0x1c0043c in getRowAndCell (ghostty)
        assert(y < self.size.rows);
              ^
/home/tim/repos/ghostty/src/terminal/PageList.zig:2472:49: 0x1b3bf09 in rowAndCell (ghostty)
        const rac = self.page.data.getRowAndCell(self.x, self.y);
                                                ^
/home/tim/repos/ghostty/src/terminal/Screen.zig:450:53: 0x1c5bd2b in cursorReload (ghostty)
    const page_rac = self.cursor.page_pin.rowAndCell();
                                                    ^
/home/tim/repos/ghostty/src/terminal/Screen.zig:854:22: 0x1dd5faf in resizeInternal (ghostty)
    self.cursorReload();
                     ^
/home/tim/repos/ghostty/src/terminal/Screen.zig:812:28: 0x1dd6043 in resize (ghostty)
    try self.resizeInternal(cols, rows, true);
                           ^
/home/tim/repos/ghostty/src/terminal/Terminal.zig:2060:35: 0x1dd6490 in resize (ghostty)
            try self.screen.resize(cols, rows);
                                  ^
/home/tim/repos/ghostty/src/termio/Exec.zig:435:33: 0x1dd6cef in resize (ghostty)
        try self.terminal.resize(
                                ^
/home/tim/repos/ghostty/src/termio/Thread.zig:353:25: 0x1de86bc in callback (ghostty)
        self.impl.resize(v.grid_size, v.screen_size, v.padding) catch |err| {
                        ^
/home/tim/.cache/zig/p/12203116ff408eb48f81c947dfeb06f7feebf6a9efa962a560ac69463098b2c04a96/src/backend/io_uring.zig:768:29: 0x1cfe701 in invoke (ghostty)
        return self.callback(self.userdata, loop, self, result);
                            ^
/home/tim/.cache/zig/p/12203116ff408eb48f81c947dfeb06f7feebf6a9efa962a560ac69463098b2c04a96/src/backend/io_uring.zig:159:33: 0x1d01300 in tick___anon_155654 (ghostty)
                switch (c.invoke(self, cqe.res)) {
                                ^
/home/tim/.cache/zig/p/12203116ff408eb48f81c947dfeb06f7feebf6a9efa962a560ac69463098b2c04a96/src/backend/io_uring.zig:60:42: 0x1d0149d in run (ghostty)
            .until_done => try self.tick_(.until_done),
                                         ^
/home/tim/repos/ghostty/src/termio/Thread.zig:236:22: 0x1d0593a in threadMain_ (ghostty)
    try self.loop.run(.until_done);
                     ^
/home/tim/repos/ghostty/src/termio/Thread.zig:140:21: 0x1cb7421 in threadMain (ghostty)
    self.threadMain_() catch |err| {
                    ^
/home/tim/.zig/versions/zig-linux-x86_64-0.12.0-dev.3336+dbb11915b/lib/std/Thread.zig:406:13: 0x1c751ea in callFn__anon_151172 (ghostty)
            @call(.auto, f, args);
            ^
/home/tim/.zig/versions/zig-linux-x86_64-0.12.0-dev.3336+dbb11915b/lib/std/Thread.zig:674:30: 0x1c18d02 in entryFn (ghostty)
                return callFn(f, args_ptr.*);

@mitchellh
Copy link
Contributor

mitchellh commented Mar 18, 2024

@rockorager Your issue is fixed: 4fe49c7 unless there was more to it, but I reproduced an issue moving tiled windows around very easily and can't get another crash yet.

@mitchellh
Copy link
Contributor

mitchellh commented Mar 18, 2024

Update: I fixed a number of issues regarding making font size tiny. There is still more work to do on there, but the "big pager" issue is the one I have in my sights right now. On the path I already found two broken assertions, and I'm aware of at least two more.

The core issue is we're resizing the grid size to a point that doesn't fit into a "standard page size" for our memory pool and it turns out I didn't really handle this case at all in PageList. I've now covered that use case for some parts but not all. I'm adding tests as I go to trigger them and more work is required, but I've also pushed a number of commits which help the situation.

@rockorager
Copy link
Collaborator

Here's another, off the latest paged-terminal branch

/home/tim/.zig/versions/zig-linux-x86_64-0.12.0-dev.3336+dbb11915b/lib/std/debug.zig:403:14: 0x1a630ac in assert (ghostty)
    if (!ok) unreachable; // assertion failure
             ^
/home/tim/repos/ghostty/src/terminal/size.zig:39:19: 0x1b3c4e2 in ptr__anon_116633 (ghostty)
            assert(addr % @alignOf(T) == 0);
                  ^
/home/tim/repos/ghostty/src/terminal/page.zig:250:50: 0x1c599cc in cloneFrom (ghostty)
            const other_cells = src_row.cells.ptr(other.memory)[0..cell_len];
                                                 ^
/home/tim/repos/ghostty/src/terminal/PageList.zig:380:32: 0x1d608a8 in clone (ghostty)
        try page.data.cloneFrom(
                               ^
/home/tim/repos/ghostty/src/terminal/Screen.zig:226:37: 0x1d63124 in clonePool (ghostty)
    var pages = try self.pages.clone(.{
                                    ^
/home/tim/repos/ghostty/src/terminal/Screen.zig:208:30: 0x1d63fca in clone (ghostty)
    return try self.clonePool(alloc, null, top, bot);
                             ^
/home/tim/repos/ghostty/src/renderer/OpenGL.zig:693:58: 0x1d8fb23 in updateFrame (ghostty)
        var screen_copy = try state.terminal.screen.clone(
                                                         ^
/home/tim/repos/ghostty/src/renderer/Thread.zig:463:27: 0x1dd03a8 in callback (ghostty)
    t.renderer.updateFrame(
                          ^
/home/tim/.cache/zig/p/12203116ff408eb48f81c947dfeb06f7feebf6a9efa962a560ac69463098b2c04a96/src/backend/io_uring.zig:768:29: 0x1cfeb81 in invoke (ghostty)
        return self.callback(self.userdata, loop, self, result);
                            ^
/home/tim/.cache/zig/p/12203116ff408eb48f81c947dfeb06f7feebf6a9efa962a560ac69463098b2c04a96/src/backend/io_uring.zig:159:33: 0x1d01780 in tick___anon_155660 (ghostty)
                switch (c.invoke(self, cqe.res)) {
                                ^
/home/tim/.cache/zig/p/12203116ff408eb48f81c947dfeb06f7feebf6a9efa962a560ac69463098b2c04a96/src/backend/io_uring.zig:60:42: 0x1d0191d in run (ghostty)
            .until_done => try self.tick_(.until_done),
                                         ^
/home/tim/repos/ghostty/src/renderer/Thread.zig:212:26: 0x1d01b22 in threadMain_ (ghostty)
    _ = try self.loop.run(.until_done);
                         ^
/home/tim/repos/ghostty/src/renderer/Thread.zig:174:21: 0x1cb78b5 in threadMain (ghostty)
    self.threadMain_() catch |err| {
                    ^
/home/tim/.zig/versions/zig-linux-x86_64-0.12.0-dev.3336+dbb11915b/lib/std/Thread.zig:406:13: 0x1c7570a in callFn__anon_151177 (ghostty)
            @call(.auto, f, args);
            ^
/home/tim/.zig/versions/zig-linux-x86_64-0.12.0-dev.3336+dbb11915b/lib/std/Thrx1c18792 in entryFn (ghostty)                                                                                  return callFn(f, args_ptr.*);                                                                               ^                                                                 ???:?:?: 0x7f1cae1dc896 in ??? (libc.so.6)                                                     Unwind information for `libc.so.6:0x7f1cae1dc896` was not available, trace may be incomplete                                                                                                  ???:?:?: 0x7f1cae26380b in ??? (libc.so.6)                                                     run                                                                                            └─ run ghostty failure                                                                         error: the following command terminated unexpectedly:                                          /home/tim/repos/ghostty/zig-cache/o/fbc751c6ddff3592d73bc47a724b52fc/ghostty                   Build Summary: 43/45 steps succeeded; 1 failed (disable with --summary none)                   run transitive failure                                                                         └─ run ghostty failure                                                                         error: the following build command failed with exit code 1:                                    /home/tim/repos/ghostty/zig-cache/o/307fae6aa308999e58f6d78e54db693e/build /home/tim/.zig/versions/zig-linux-x86_64-0.12.0-dev.3336+dbb11915b/zig /home/tim/repos/ghostty /home/tim/repos/ghostty/zig-cache /home/tim/.cache/zig --seed 0xd0df26a3 -Z8a27f137476f12d5 run

@mitchellh
Copy link
Contributor

Big Pager fixed: 522867c

@mitchellh
Copy link
Contributor

I believe all the original issues reported by @qwerasd205 are fixed. There is still one consistent cat /dev/urandom crash and I'll use this issue to track it. No repro yet.

/Users/mitchellh/code/go/src/github.com/mitchellh/ghostty/src/terminal/Terminal.zig:1426:46: 0x102dcb323 in deleteLines (ghostty)
    const clear_top = top.down(scroll_amount).?;
                                             ^
/Users/mitchellh/code/go/src/github.com/mitchellh/ghostty/src/termio/Exec.zig:2043:34: 0x102edf88b in deleteLines (ghostty)
        self.terminal.deleteLines(count);
                                 ^
/Users/mitchellh/code/go/src/github.com/mitchellh/ghostty/src/terminal/stream.zig:555:54: 0x102eeba6b in csiDispatch (ghostty)
                    1 => try self.handler.deleteLines(action.params[0]),
                                                     ^
/Users/mitchellh/code/go/src/github.com/mitchellh/ghostty/src/terminal/stream.zig:287:71: 0x102eff793 in nextNonUtf8 (ghostty)
                    .csi_dispatch => |csi_action| try self.csiDispatch(csi_action),
                                                                      ^
/Users/mitchellh/code/go/src/github.com/mitchellh/ghostty/src/terminal/stream.zig:141:37: 0x102f00ae3 in consumeUntilGround (ghostty)
                try self.nextNonUtf8(input[offset]);
                                    ^
/Users/mitchellh/code/go/src/github.com/mitchellh/ghostty/src/terminal/stream.zig:129:54: 0x102f00dab in consumeAllEscapes (ghostty)
                offset += try self.consumeUntilGround(input[offset..]);
                                                     ^
/Users/mitchellh/code/go/src/github.com/mitchellh/ghostty/src/terminal/stream.zig:114:53: 0x102f01aff in nextSliceCapped (ghostty)
                offset += try self.consumeAllEscapes(input[offset..]);
                                                    ^
/Users/mitchellh/code/go/src/github.com/mitchellh/ghostty/src/terminal/stream.zig:60:41: 0x102f01dc3 in nextSlice (ghostty)
                try self.nextSliceCapped(input[i .. i + len], &cp_buf);
                                        ^
/Users/mitchellh/code/go/src/github.com/mitchellh/ghostty/src/termio/Exec.zig:1634:41: 0x102eadd3b in threadMainPosix (ghostty)
            ev.terminal_stream.nextSlice(buf) catch |err|
                                        ^
/nix/store/y1rw1hcxc1v6d4l6cjvxxpvgcgfmxi66-zig-0.12.0-dev.3282+da5b16f9e/lib/std/Thread.zig:406:13: 0x102e427bb in callFn__anon_266058 (ghostty)
            @call(.auto, f, args);
            ^
/nix/store/y1rw1hcxc1v6d4l6cjvxxpvgcgfmxi66-zig-0.12.0-dev.3282+da5b16f9e/lib/std/Thread.zig:674:30: 0x102dca8ff in entryFn (ghostty)
                return callFn(f, args_ptr.*);

@KNnut
Copy link
Collaborator

KNnut commented Mar 19, 2024

The latest paged-terminal branch (cae1f0f), when using neovim

thread panic: attempt to use null value
src/terminal/page.zig:282:91: 0x19be67a in cloneRowFrom (ghostty)
                const other_style = other.styles.lookupId(other.memory, src_cell.style_id).?.*;
                                                                                          ^
src/terminal/page.zig:243:74: 0x19bf7ee in cloneFrom (ghostty)
        for (rows, other_rows) |*dst_row, *src_row| try self.cloneRowFrom(
                                                                         ^
src/terminal/PageList.zig:380:32: 0x1ac8500 in clone (ghostty)
        try page.data.cloneFrom(
                               ^
src/terminal/Screen.zig:226:37: 0x1acae14 in clonePool (ghostty)
    var pages = try self.pages.clone(.{
                                    ^
src/terminal/Screen.zig:208:30: 0x1acbcda in clone (ghostty)
    return try self.clonePool(alloc, null, top, bot);
                             ^
src/renderer/OpenGL.zig:693:58: 0x1af7833 in updateFrame (ghostty)
        var screen_copy = try state.terminal.screen.clone(
                                                         ^
src/renderer/Thread.zig:463:27: 0x1b38248 in callback (ghostty)
    t.renderer.updateFrame(
                          ^
~/.cache/zig/p/12203116ff408eb48f81c947dfeb06f7feebf6a9efa962a560ac69463098b2c04a96/src/backend/io_uring.zig:768:29: 0x1a64f91 in invoke (ghostty)
        return self.callback(self.userdata, loop, self, result);
                            ^
~/.cache/zig/p/12203116ff408eb48f81c947dfeb06f7feebf6a9efa962a560ac69463098b2c04a96/src/backend/io_uring.zig:159:33: 0x1a67b90 in tick___anon_155952 (ghostty)
                switch (c.invoke(self, cqe.res)) {
                                ^
~/.cache/zig/p/12203116ff408eb48f81c947dfeb06f7feebf6a9efa962a560ac69463098b2c04a96/src/backend/io_uring.zig:60:42: 0x1a67d2d in run (ghostty)
            .until_done => try self.tick_(.until_done),
                                         ^
src/renderer/Thread.zig:212:26: 0x1a67f32 in threadMain_ (ghostty)
    _ = try self.loop.run(.until_done);
                         ^
src/renderer/Thread.zig:174:21: 0x1a1d125 in threadMain (ghostty)
    self.threadMain_() catch |err| {
                    ^
/usr/lib/zig/std/Thread.zig:406:13: 0x19da5ba in callFn__anon_151468 (ghostty)
            @call(.auto, f, args);
            ^
/usr/lib/zig/std/Thread.zig:674:30: 0x197cf22 in entryFn (ghostty)
                return callFn(f, args_ptr.*);
                             ^

Repro

With this init.lua

local lazypath = vim.fn.stdpath('data') .. '/lazy/lazy.nvim'
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({
    'git',
    'clone',
    '--filter=blob:none',
    'https://github.com/folke/lazy.nvim',
    lazypath,
  })
end
vim.opt.rtp:prepend(lazypath)

require('lazy').setup({
    {
        'catppuccin/nvim',
        name = 'catppuccin',
        priority = 1000,
        config = function()
            vim.cmd.colorscheme 'catppuccin'
        end
    },
    {
        'L3MON4D3/LuaSnip',
        config = function()
            require('luasnip.loaders.from_vscode').lazy_load()
        end,
        dependencies = {
            'rafamadriz/friendly-snippets'
        }
    },
    {
        'hrsh7th/nvim-cmp',
        dependencies = {
            'saadparwaiz1/cmp_luasnip',
        },
        config = function()
            local cmp = require('cmp')
            cmp.setup({
                sources = cmp.config.sources({
                    { name = 'luasnip' }
                })
            })
        end
    }
})
  1. Type an emoji (like 😀) or CJK character (like 一) in insert mode
  2. Return to normal mode
  3. Shift+o and type d

A popup menu with catppuccin theme causes the crash.

@mitchellh
Copy link
Contributor

@KNnut I can't reproduce it in neovim but I do have a reproduction in another way with an identical stack trace so I think I can get this fixed.

@mitchellh
Copy link
Contributor

@KNnut fixed that.

@mitchellh
Copy link
Contributor

Going to close this issue. All crashes in this issue are now fixed afaict. If there are new ones, please open a new issue.

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

No branches or pull requests

4 participants