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

Add _NET_WM_PID atom to Linux X11 window #17470

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

lindexi
Copy link
Contributor

@lindexi lindexi commented Nov 11, 2024

What does the pull request do?

Append the _NET_WM_PID atom to Linux X11 window.

I learn from gtk3:

// https://gitlab.gnome.org/GNOME/gtk/blob/3.24.1/gdk/x11/gdkwindow-x11.c#L935-944
  if (!gdk_running_in_sandbox ())
    {
      /* if sandboxed, we're likely in a pid namespace and would only confuse the wm with this */
      pid_t pid = getpid ();
      XChangeProperty (xdisplay, xid,
                       gdk_x11_get_xatom_by_name_for_display (x11_screen->display, "_NET_WM_PID"),
                       XA_CARDINAL, 32,
                       PropModeReplace,
                       (guchar *)&pid, 1);
    }

And I learn from qt:

// https://github.com/qt/qtbase/blob/44876fb45e702c2554fca98ed19af57feb1c0511/src/plugins/platforms/xcb/qxcbwindow.cpp#L434-L445
    quint32 pid = getpid();
    xcb_change_property(xcb_connection(), XCB_PROP_MODE_REPLACE, m_window,
                        atom(QXcbAtom::Atom_NET_WM_PID), XCB_ATOM_CARDINAL, 32,
                        1, &pid);

    const QByteArray clientMachine = QSysInfo::machineHostName().toLocal8Bit();
    if (!clientMachine.isEmpty()) {
        xcb_change_property(xcb_connection(), XCB_PROP_MODE_REPLACE, m_window,
                            atom(QXcbAtom::AtomWM_CLIENT_MACHINE), XCB_ATOM_STRING, 8,
                            clientMachine.size(), clientMachine.constData());
    }

// https://github.com/qt/qtbase/blob/44876fb45e702c2554fca98ed19af57feb1c0511/src/corelib/global/qsysinfo.cpp#L948-L957
QString QSysInfo::machineHostName()
{
    // the hostname can change, so we can't cache it
#if defined(Q_OS_LINUX)
    // gethostname(3) on Linux just calls uname(2), so do it ourselves
    // and avoid a memcpy
    struct utsname u;
    if (uname(&u) == 0)
        return QString::fromLocal8Bit(u.nodename);
    return QString();
}

What is the current behavior?

Miss the _NET_WM_PID atom in Avalonia Linux x11 window.

What is the updated/expected behavior with this PR?

We can find the _NET_WM_PID atom in Avalonia Linux x11 window. And we get the the Process Id from Avalonia Linux x11 window by _NET_WM_PID atom.

How was the solution implemented (if it's not obvious)?

Checklist

Breaking changes

Obsoletions / Deprecations

Fixed issues

Fixes #17444

@lindexi lindexi marked this pull request as ready for review November 11, 2024 07:01
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0053207-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]


// If _NET_WM_PID is set, the ICCCM-specified property WM_CLIENT_MACHINE MUST also be set.
// the hostname can change, so we can't cache it
// gethostname(3) on Linux just calls uname(2), so do it ourselves
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we just P/Invoke gethostname instead of that complex machinery in GetUtsName?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kekekeks I copy from qt:

// https://github.com/qt/qtbase/blob/44876fb45e702c2554fca98ed19af57feb1c0511/src/corelib/global/qsysinfo.cpp#L948-L957
QString QSysInfo::machineHostName()
{
    // the hostname can change, so we can't cache it
#if defined(Q_OS_LINUX)
    // gethostname(3) on Linux just calls uname(2), so do it ourselves
    // and avoid a memcpy
    struct utsname u;
    if (uname(&u) == 0)
        return QString::fromLocal8Bit(u.nodename);
    return QString();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather have an extra memcpy than complex piece of interop code.

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0053460-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

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

Successfully merging this pull request may close these issues.

Add _NET_WM_PID atom to Linux X11 window
3 participants