Commit 304305f7 authored by Waqar Ahmed's avatar Waqar Ahmed Committed by Tomaz Canabrava
Browse files

Dont resize screenLines all the time

parent fbf48758
......@@ -936,7 +936,6 @@ void Screen::displayCharacter(uint c)
// ensure current line vector has enough elements
if (_screenLines[_cuY].size() < _cuX + w) {
// _screenLines[_cuY].reserve(_columns);
_screenLines[_cuY].resize(_cuX + w);
}
......@@ -1117,7 +1116,8 @@ void Screen::clearImage(int loca, int loce, char c, bool resetLineRendition)
Character clearCh(uint(c), _currentForeground, _currentBackground, DEFAULT_RENDITION, false);
// if the character being used to clear the area is the same as the
// default character, the affected _lines can simply be shrunk.
// default character, the affected _lines can simply be filled
// with the default char
const bool isDefaultCh = (clearCh == Screen::DefaultChar);
for (int y = topLine; y <= bottomLine; ++y) {
......@@ -1129,7 +1129,7 @@ void Screen::clearImage(int loca, int loce, char c, bool resetLineRendition)
QVector<Character> &line = _screenLines[y];
if (isDefaultCh && endCol == _columns - 1) {
line.resize(startCol);
std::fill(line.begin() + startCol, line.end(), clearCh);
  • This is segfaulting! @tcanabrava @waqar .

  • (gdb) l
    907         __gnu_cxx::__enable_if<!__is_scalar<_Tp>::__value, void>::__type
    908         __fill_a1(_ForwardIterator __first, _ForwardIterator __last,
    909                   const _Tp& __value)
    910         {
    911           for (; __first != __last; ++__first)
    912             *__first = __value;
    913         }
    914
    915       template<typename _ForwardIterator, typename _Tp>
    916         _GLIBCXX20_CONSTEXPR
    (gdb) p __first
    $28 = {i = 0x555556068ff8}
    (gdb) p *__first
    $29 = (Konsole::Character &) <error reading variable>
    (gdb) p __value
    $30 = (const Konsole::Character &) @0x7fffffffd3f0: {character = 32, rendition = 0, foregroundColor = {_colorSpace = 1 '\001', _u = 0 '\000', 
        _v = 0 '\000', _w = 0 '\000'}, backgroundColor = {_colorSpace = 1 '\001', _u = 1 '\001', _v = 0 '\000', _w = 0 '\000'}, 
      isRealCharacter = false}
    (gdb) p __last
    $31 = {i = 0x555555fadda8}
    (gdb) p __first
    $32 = {i = 0x555556068ff8}
    (gdb) up
    #1  0x00007ffff7cfe4a3 in std::__fill_a<QTypedArrayData<Konsole::Character>::iterator, Konsole::Character> (__first=..., __last=..., __value=...)
        at /usr/include/c++/11.1.0/bits/stl_algobase.h:969
    969         { std::__fill_a1(__first, __last, __value); }
    (gdb) l
    964
    965       template<typename _FIte, typename _Tp>
    966         _GLIBCXX20_CONSTEXPR
    967         inline void
    968         __fill_a(_FIte __first, _FIte __last, const _Tp& __value)
    969         { std::__fill_a1(__first, __last, __value); }
    970
    971       template<typename _Ite, typename _Seq, typename _Cat, typename _Tp>
    972         void
    973         __fill_a(const ::__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat>&,
    (gdb) up
    #2  0x00007ffff7cfb54c in std::fill<QTypedArrayData<Konsole::Character>::iterator, Konsole::Character> (__first=..., __last=..., __value=...)
        at /usr/include/c++/11.1.0/bits/stl_algobase.h:999
    999           std::__fill_a(__first, __last, __value);
    (gdb) l
    994           // concept requirements
    995           __glibcxx_function_requires(_Mutable_ForwardIteratorConcept<
    996                                       _ForwardIterator>)
    997           __glibcxx_requires_valid_range(__first, __last);
    998
    999           std::__fill_a(__first, __last, __value);
    1000        }
    1001
    1002      // Used by fill_n, generate_n, etc. to convert _Size to an integral type:
    1003      inline _GLIBCXX_CONSTEXPR int
    (gdb) up
    #3  0x00007ffff7cf5efc in Konsole::Screen::clearImage (this=0x555555853770, loca=8492, loce=8509, c=32 ' ', resetLineRendition=false)
        at /home/hematite/Documents/Build/konsole/src/Screen.cpp:1132
    1132                std::fill(line.begin() + startCol, line.end(), clearCh);
    (gdb) l
    1127            const int startCol = (y == topLine) ? loca % _columns : 0;
    1128
    1129            QVector<Character> &line = _screenLines[y];
    1130
    1131            if (isDefaultCh && endCol == _columns - 1) {
    1132                std::fill(line.begin() + startCol, line.end(), clearCh);
    1133            } else {
    1134                if (line.size() < endCol + 1) {
    1135                    line.resize(endCol + 1);
    1136                }
    (gdb) p line
    $33 = (QVector<Konsole::Character> &) @0x555555bd8748: {d = 0x555555fadba0}
    (gdb) l
    1137
    1138                if (endCol == _columns - 1) {
    1139                    line.resize(endCol + 1);
    1140                }
    1141
    1142                if (startCol <= endCol) {
    1143                    std::fill(line.begin() + startCol, line.begin() + (endCol + 1), clearCh);
    1144                }
    1145            }
    1146
    (gdb) p line.size
    $36 = {int (const QVector<Konsole::Character> * const)} 0x7ffff7cf9912 <QVector<Konsole::Character>::size() const>
    (gdb) p line.size()
    $37 = 31
    (gdb) 
  • It might not be this commit that's causing it! It might just be showing up as a result of an earlier commit.

Please register or sign in to reply
} else {
if (line.size() < endCol + 1) {
line.resize(endCol + 1);
......@@ -1164,10 +1164,8 @@ void Screen::moveImage(int dest, int sourceBegin, int sourceEnd)
const int destY = dest / _columns;
const int srcY = sourceBegin / _columns;
if (dest < sourceBegin) {
for (int i = 0; i <= lines; ++i) {
_screenLines[destY + i] = _screenLines.at(srcY + i);
_lineProperties[destY + i] = _lineProperties.at(srcY + i);
}
std::rotate(_screenLines.begin() + destY, _screenLines.begin() + srcY, _screenLines.end());
std::rotate(_lineProperties.begin() + destY, _lineProperties.begin() + srcY, _lineProperties.end());
} else {
for (int i = lines; i >= 0; --i) {
_screenLines[destY + i] = std::move(_screenLines[srcY + i]);
......@@ -1617,7 +1615,12 @@ void Screen::fastAddHistLine()
_escapeSequenceUrlExtractor->historyLinesRemoved(1);
}
_screenLines.erase(_screenLines.begin());
// Rotate left + clear the last line
std::rotate(_screenLines.begin(), _screenLines.begin() + 1, _screenLines.end());
auto last = _screenLines.back();
Character clearCh(uint(' '), _currentForeground, _currentBackground, DEFAULT_RENDITION, false);
std::fill(last.begin(), last.end(), clearCh);
_lineProperties.erase(_lineProperties.begin());
}
......
  • Konsole::Screen::clearImage > std::fill is occasionally (after a few minutes of use) segfaulting after a recent commit (possibly not this commit). See https://bugs.kde.org/show_bug.cgi?id=445550 .

    Edited by Henry Heino
  • mentioned in merge request !516 (merged)

    Toggle commit list
  • @personalizedrefrigerator Can you maybe share what you were doing before it crashes? I have been using this change for a while without any crashes so it must be something that I don't do.

  • It seems somewhat random... I was using Vim to write a LaTeX document most of the times that it crashed.

    See the reproduction steps associated with the bug for what I was (generally) doing.

  • Please note that this could be the result of something else that was merged today. I can try to find the specific commit (but it might take me a few days to get to this).

    Edit: If I open vim, enter edit mode, and hold down enter, I can quickly reproduce the crash. My .vimrc can be found here: https://github.com/personalizedrefrigerator/dotfiles/blob/main/.vimrc

    Edit 2: I am most suspicious of these lines in my .vimrc:

    " Change cursor when entering insert mode
    " Ref: https://stackoverflow.com/questions/6488683/how-do-i-change-the-cursor-between-normal-and-insert-modes-in-vim
    let &t_SI = "\e[6 q"
    let &t_EI = "\e[2 q"
    Edited by Henry Heino
  • Okay, found another issue that is related (moveImage was broken). Fixing that, I think that will fix the crash too.

  • @personalizedrefrigerator please try with latest master, see if you can still make it crash. Also, vim :help was broken, so that should be working correctly too now. Thanks a lot for bringing this up so early!

  • I haven't been able to reproduce this on master — I think it's fixed!

  • I'm getting crashes too with yesterday's git master in vim, when hitting the delete or backspace key at the end of a line while writing a commit message. Will try again with today's git master.

  • Still seeing it with today's git master. :(

  • @ngraham I tried reproducing it but seems to be working for me. Do you have more detailed repro instructions maybe?

  • I don't sorry. Just what I mentioned already. I couldn't find a 100% consistent reproducer.

    That said, it's stopped happening. Maybe it got fixed with some of the recent changes yesterday. and the day before. I'll keep an eye on this.

  • I kept trying to reproduce this today at work, but haven't been able to.

    In any case, will be happy to know if you reproduce it again. We need our Konsole to be invincible ;)

    Edited by Waqar Ahmed
  • Indeed!

Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment