Bug 367746

Probably started with commit 2f1c8cf397be6c91f and commit c6c09ce7a38fccd.
Please revert.
Thanks.

The alternative is data loss because the active process terminates its children, as explained in https://bugs.kde.org/show_bug.cgi?id=185140

As the bash maintainer explained EOF is the correct way for a terminal emulator to end a session.

If you have a better solution that doesn't lead to data loss I'm all ears.

Created attachment 100811 [details]
Only send EOF if no foreground process

Can you test this patch and see if it helps for your usecase?

(In reply to Martin Sandsmark from comment #2)
> The alternative is data loss because the active process terminates its
> children, as explained in https://bugs.kde.org/show_bug.cgi?id=185140
> 
> As the bash maintainer explained EOF is the correct way for a terminal
> emulator to end a session.
> 
> If you have a better solution that doesn't lead to data loss I'm all ears.

One could argue that this isn't the job of an terminal emulator.
The user can specify the desired behavior in his shell config. So there
really is no need to change the way Konsole behaves.

Anyway your patch seems to fix the issue for me.
Thanks.

(In reply to Markus Trippelsdorf from comment #4)
> (In reply to Martin Sandsmark from comment #2)
> > The alternative is data loss because the active process terminates its
> > children, as explained in https://bugs.kde.org/show_bug.cgi?id=185140
> > 
> > As the bash maintainer explained EOF is the correct way for a terminal
> > emulator to end a session.
> > 
> > If you have a better solution that doesn't lead to data loss I'm all ears.
> 
> One could argue that this isn't the job of an terminal emulator.
> The user can specify the desired behavior in his shell config. So there
> really is no need to change the way Konsole behaves.
> 
> Anyway your patch seems to fix the issue for me.
> Thanks.

No, I spoke too soon. The issue still happens (but not always) with your patch.

Ok, then I'll have to track the shell process in some other way, probably by PID. The idea is to only send EOF if the only process running is the initially launched shell, not if e. g. screen or top is running. screen broke it somewhat because I just tracked the current process by name, and obviously bash is the active process in the screen.

I think that should be a good solution for everyone.

I'm not sure how this is supposed to work.

After boot I have, e.g.:  
  |-login,221 -f       
  |   `-zsh,227
  |       `-startx,228 /usr/bin/startx
  |           `-xinit,244 /home/markus/.xinitrc -- /etc/X11/xinit/xserverrc :0 -auth /home/markus/.serverauth.228
  |               |-X,245 -nolisten tcp :0 -auth /home/markus/.serverauth.228
  |               |  `-{radeon_cs:0},248
  |               `-xmonad-x86_64-l,250
  |                   |-konsole,278 -e /home/markus/start_tmux
  |                   |   |-start_tmux,300 /home/markus/start_tmux
  |                   |   |   `-tmux,302 attach-session
  |                   |   |-{QDBusConnection},293
  |                   |   |-{QXcbEventReader},292
  |                   |   `-{radeon_cs:0},299

After closing and re-attaching this gets:
  ├─login,221 -f       
  │   └─zsh,227
  │       └─startx,228 /usr/bin/startx
  │           └─xinit,244 /home/markus/.xinitrc -- /etc/X11/xinit/xserverrc :0 -auth /home/markus/.serverauth.228
  │               ├─X,245 -nolisten tcp :0 -auth /home/markus/.serverauth.228
  │               │  └─{radeon_cs:0},248
  │               └─xmonad-x86_64-l,250
  │                   ├─konsole,435
  │                   │   ├─zsh,440
  │                   │   │   └─tmux,445 attach-session
  │                   │   ├─zsh,505
  │                   │   │   └─ssh,510 trippels@gcc112.fsffrance.org
  │                   │   ├─{QDBusConnection},437
  │                   │   ├─{QXcbEventReader},436
  │                   │   └─{radeon_cs:0},439
  │                   ├─unclutter,280 -noevents -idle 1
  │                   ├─xmobar,282
  │                   └─xscreensaver,274

Where "ssh,510 trippels@gcc112.fsffrance.org" is a connection to a remote tmux session
in a new konsole window.

"tmux,445 attach-session" is the local tmux session:

  ├─tmux,304 attach-session
  │   ├─mutt,316
  │   ├─ncmpcpp,314
  │   ├─sh,309 /home/markus/multitail
  │   │   └─sudo,374 multitail --no-mark-change -M 0 -csn /var/log/messages -cS kernel /var/log/kern.log
  │   │       └─multitail,375 --no-mark-change -M 0 -csn /var/log/messages -cS kernel /var/log/kern.log
  │   │           ├─tail,376 --follow=name -n 157 /var/log/messages
  │   │           └─tail,377 --follow=name -n 50 /var/log/kern.log
  │   ├─sudo,305 su
  │   │   └─su,312
  │   │       └─zsh,332
  │   ├─sudo,307 su
  │   │   └─su,313
  │   │       └─zsh,333
  │   ├─zsh,318
  │   ├─zsh,320
  │   ├─zsh,324
  │   ├─zsh,326
  │   └─zsh,329
  │       └─pstree,1325 -apl

Created attachment 100817 [details]
Use proper methods to check for active PID

This works in my testing.

(In reply to Martin Sandsmark from comment #8)
> Created attachment 100817 [details]
> Use proper methods to check for active PID
> 
> This works in my testing.

It crashes when re-attaching a tmux session:

Thread 1 "konsole" received signal SIGSEGV, Segmentation fault.
Konsole::ProcessInfo::name (this=this@entry=0x0, ok=ok@entry=0x7fffffffd9af) at /var/tmp/konsole/src/ProcessInfo.cpp:231
231         *ok = _fields.testFlag(NAME);
(gdb) bt
#0  Konsole::ProcessInfo::name (this=this@entry=0x0, ok=ok@entry=0x7fffffffd9af) at /var/tmp/konsole/src/ProcessInfo.cpp:231
#1  0x00007ffff7f04b5d in Konsole::Session::getDynamicTitle (this=0x4b3290) at /var/tmp/konsole/src/Session.cpp:1058
#2  0x00007ffff7f11829 in Konsole::SessionController::snapshot (this=0x660f30) at /var/tmp/konsole/src/SessionController.cpp:253
#3  0x00007ffff7f19d1d in QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (Konsole::SessionController::*)()>::call(void (Konsole::SessionController::*)(), Konsole::SessionController*, void**) (arg=<optimized out>, o=<optimized out>, f=<optimized out>) at /usr/include/qt5/QtCore/qobjectdefs_impl.h:507
#4  QtPrivate::FunctionPointer<void (Konsole::SessionController::*)()>::call<QtPrivate::List<>, void>(void (Konsole::SessionController::*)(), Konsole::SessionController*, void**)
    (arg=<optimized out>, o=<optimized out>, f=<optimized out>) at /usr/include/qt5/QtCore/qobjectdefs_impl.h:526
#5  QtPrivate::QSlotObject<void (Konsole::SessionController::*)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (
    which=<optimized out>, this_=<optimized out>, r=<optimized out>, a=<optimized out>, ret=<optimized out>) at /usr/include/qt5/QtCore/qobject_impl.h:149
#6  0x00007ffff61fbced in QMetaObject::activate(QObject*, int, int, void**) () from /usr/lib/libQt5Core.so.5
#7  0x00007ffff6208728 in QTimer::timerEvent(QTimerEvent*) () from /usr/lib/libQt5Core.so.5
#8  0x00007ffff61fc8cb in QObject::event(QEvent*) () from /usr/lib/libQt5Core.so.5
#9  0x00007ffff6aefe9c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/libQt5Widgets.so.5
#10 0x00007ffff6af7174 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/libQt5Widgets.so.5
#11 0x00007ffff61d0000 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/libQt5Core.so.5
#12 0x00007ffff6222f0e in QTimerInfoList::activateTimers() () from /usr/lib/libQt5Core.so.5
#13 0x00007ffff6223481 in timerSourceDispatch(_GSource*, int (*)(void*), void*) () from /usr/lib/libQt5Core.so.5
#14 0x00007ffff43b9dce in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#15 0x00007ffff43ba038 in g_main_context_iterate.isra () from /usr/lib/libglib-2.0.so.0
#16 0x00007ffff43ba0ec in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#17 0x00007ffff6223fdf in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt5Core.so.5
#18 0x00007ffff61ce44a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/libQt5Core.so.5
#19 0x00007ffff61d67ed in QCoreApplication::exec() () from /usr/lib/libQt5Core.so.5
#20 0x00007ffff7fd2a9d in kdemain (argc=<optimized out>, argv=<optimized out>) at /var/tmp/konsole/src/main.cpp:174
#21 0x0000000000400a9d in main (argc=<optimized out>, argv=<optimized out>) at /var/tmp/konsole/build/src/konsole_dummy.cpp:3

Created attachment 100818 [details]
Patch #3

Try this. There were some implicit dependencies on side-effects...

Seems to work now. Thanks.
BTW your:
commit 04dce67996a9318494c79c2617e3c88620e8e19e
Author: Martin T. H. Sandsmark <martin.sandsmark@kde.org>
Date:   Sat Aug 27 14:38:26 2016 +0200

    Make the URL hint keyboard modifiers configurable
    
    Instead of hardcoding CTRL which was a bit annoying add checkboxes to
    the profile edit dialog to let the user select which keys to use.
    
    REVIEW: 128778

Creates strange visible distortions. When I type something in my shell, completely unrelated glyphs from older output get changed in strange ways. And konsole consumes a lot of CPU and general response time is unbearable.

And it still doesn't fully work unfortunately.
When I start tmux for the first time, then close Konsole, then re-attach:
the last active shell window is now gone.

I have "konsole -e "tmux attach-session" > /dev/null 2>&1 &" in my .xinitrc,
so there is no underlying shell in this case.

Ah, the -e messes it up indeed.

Ok, I'll push the first patch anyways, it makes things a bit tidier, and then I'll just check the name of the base process against a list of known shells, I guess. Unless I come up with a better idea.

Git commit 79ca76a94696eda408927b122b0b531676d4ba93 by Martin T. H. Sandsmark.
Committed on 28/08/2016 at 17:12.
Pushed by sandsmark into branch 'master'.

Fix checking of foreground process

The old method of checking it has unnecessary overhead (doing a full
process info update), and wasn't very reliable.

Instead just get the original shell PID from QProcess (via KProcess),
and the foreground process from the PTY.
REVIEW: 128789

M  +2    -1    src/MainWindow.cpp
M  +5    -4    src/Session.cpp

http://commits.kde.org/konsole/79ca76a94696eda408927b122b0b531676d4ba93

Created attachment 100830 [details]
Patch to only send EOF to known shells

This should fix it once and for all.

(In reply to Martin Sandsmark from comment #16)
> Created attachment 100830 [details]
> Patch to only send EOF to known shells
> 
> This should fix it once and for all.

Indeed it does. Thank you for your patience.

> Indeed it does. Thank you for your patience.

Thank you for your patience as well. :-)

https://git.reviewboard.kde.org/r/128791/

Git commit ce4ab922496cb9e133f96db78c5b67bc01429512 by Martin T. H. Sandsmark.
Committed on 28/08/2016 at 22:12.
Pushed by sandsmark into branch 'master'.

Only send EOF to known shells

The only processes that are "safe" or require us to send an EOF to
terminate cleanly are shells, so verify that people didn't run with -e
tmux or similar.

REVIEW: 128791

M  +4    -2    src/Session.cpp

http://commits.kde.org/konsole/ce4ab922496cb9e133f96db78c5b67bc01429512

Git commit 17a3ee5dfa1fc99776a015b31e5862ce82ab1a56 by Kurt Hindenburg, on behalf of Martin T. H. Sandsmark.
Committed on 31/08/2016 at 12:37.
Pushed by hindenburg into branch 'Applications/16.08'.

Fix checking of foreground process

The old method of checking it has unnecessary overhead (doing a full
process info update), and wasn't very reliable.

Instead just get the original shell PID from QProcess (via KProcess),
and the foreground process from the PTY.
REVIEW: 128789
(cherry picked from commit 79ca76a94696eda408927b122b0b531676d4ba93)

M  +2    -1    src/MainWindow.cpp
M  +5    -4    src/Session.cpp

http://commits.kde.org/konsole/17a3ee5dfa1fc99776a015b31e5862ce82ab1a56

Git commit 7cecfc078e49e1a8654a759481be31e335be2b09 by Kurt Hindenburg, on behalf of Martin T. H. Sandsmark.
Committed on 31/08/2016 at 12:38.
Pushed by hindenburg into branch 'Applications/16.08'.

Only send EOF to known shells

The only processes that are "safe" or require us to send an EOF to
terminate cleanly are shells, so verify that people didn't run with -e
tmux or similar.

REVIEW: 128791
(cherry picked from commit ce4ab922496cb9e133f96db78c5b67bc01429512)

M  +4    -2    src/Session.cpp

http://commits.kde.org/konsole/7cecfc078e49e1a8654a759481be31e335be2b09