Bug 339494 - resort to xcb_kill_client if ::kill fails
Summary: resort to xcb_kill_client if ::kill fails
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: core (other bugs)
Version First Reported In: 4.11.11
Platform: Ubuntu Linux
: NOR wishlist
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-29 02:20 UTC by Dima Ryazanov
Modified: 2024-04-25 14:26 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dima Ryazanov 2014-09-29 02:20:09 UTC
I was running "kubuntu-devel-release-upgrade", but it crashed in the middle of the upgrade. Its window is still visible and unresponsive. When I try to close it, I get the usual warning:

"""
Application "ubuntu-release-upgrader" is not responding

You tried to close window "Distribution Upgrade" from application "ubuntu-release-upgrader" (Process ID: 4176) but the application is not responding.
"""

However, process 4176 doesn't exist anymore. When I click "Terminate Application ...", nothing happens.

I finally tried "xkill", and it worked.

Reproducible: Didn't try

Steps to Reproduce:
(Don't have repro steps)
Comment 1 Christoph Feck 2014-09-29 10:28:57 UTC
The process still exists, otherwise KWin would not be able to find its name :)
Comment 2 Thomas Lübking 2014-09-29 11:38:04 UTC
Client destruction after xkill invocation may be coincidental.
Notice that kwin tries SIGTERM for local clients and uses "xon" (which i haven't even installed...) for remote ones (rather than XKillClient)

Possible reasons:
a) the client may require a more convincing SIGKILL
b) the client process may belong to a different user ("ubuntu-release-upgrader" sounds like root)
c) client indeed lost connection to server.

@Christoph
the application name is just the "resourceClass()" - we do not look up /proc (portability, i assume)
For a remote client or a completely screwed X11 server, it *would* be possible to have a window for a client with lost connection.
Since I however doubt either, I prefer answers a or b ;-)
Comment 3 Dima Ryazanov 2014-09-29 15:28:51 UTC
I forgot to mention: I ran "ps 4176" to see what process was frozen, but "ps" couldn't find it, so I'm pretty sure the process was in fact gone.
Comment 4 Thomas Lübking 2014-09-29 17:44:54 UTC
Well, we cannot "fail" to kill a process that is gone.
You didn't happen to check whether there was another PID for "ubuntu-release-upgrader", did you?
Comment 5 Dima Ryazanov 2014-09-29 18:36:45 UTC
There was a "dpkg" process still running, which might have been the cause.

I actually came up with a repro:

"""
#include <unistd.h>
#include <QApplication>
#include <QMainWindow>

int main(int argc, char *argv[]) {
  QApplication app(argc, argv);

  QMainWindow w;
  w.show();

  if (fork() == 0) {
    sleep(60);
    exit(0);
  }

  alarm(3);

  return app.exec();
}
"""

The application forks, then crashes. That's probably what ubuntu-release-upgrader did.
Comment 6 Thomas Lübking 2014-09-29 19:11:30 UTC
Many thanks.

The reason is that _NET_WM_PID isn't updated on the window for the fork(), ie. is invalid.

That's a NETWM violation, ie. client (in this case Qt) bug.

xcb_kill_client just breaks the client/server connection, but doesn't actually kill the client (though the client *may* terminate in return), so actually ::kill() is the better variant for local connections and assumed legit PIDs, so we could

if (::kill(pid, SIGTERM) != 0)
   xcb_kill_client(connection(), m_client);

BUT:
a) that doesn't necessarily kill the client
b) we might still kill the wrong process for the false PID :-\
Comment 7 Thomas Lübking 2014-09-29 19:56:32 UTC
the connection is broken, the window closed - but the demo process is still running.

My 2¢
- This is a client bug.
- If killing the client fails (other than for EPERM), we should tell the user (resolving errno) and OFFER to simply break the client/server connection, WARNING that the process will likely not be killed and suggest to open ksysguard to isolate and kill the process instead.
- Silently breaking the connection is not helpfull at all

----
https://bugreports.qt-project.org/browse/QTBUG-41664
Comment 8 Martin Flöser 2014-10-06 09:49:43 UTC
I agree with the 2 cents from previous comment. I wouldn't mind us to improve the experience, in fact we could even open the process ui filtered on the window, but that would introduce a new dependency on the library.

But I would also say that it's not a high priority as it's clearly a client bug if the pid is wrong.
Comment 9 Zamundaaa 2024-04-25 14:26:25 UTC
This has been implemented afaict