tcgetpgrp() behavior and tmux #1063

whorfin · 2016-09-07T01:39:14Z

Please use the following bug reporting template to help produce actionable and reproducible issues. Please try to ensure that the reproduction is minimal so that the team can go through more bugs!

A brief description
pane_current_path, in either the apt-get install version [1.8] or "download and build" [2.2] version of tmux is always NULL. This is because tcgetpgrp(fd) in osdep-linux.c always returns -1 error.
Using the code path from the sunos or aix versions instead, which uses ptsname(fd) to find the /dev/pts version of the tty, opens it, and then calls tcgetpgrp on that fd instead, works as expected.
Expected results
use tmux, enjoy {pane_current_path} behavior, such as the common
bind c new-window -c "#{pane_current_path}"
which opens a new window, in the current directory
Actual results (with terminal output if applicable)
use tmux, note that pane_current_path is not usable [always NULL]
Your Windows build number
Version 1607 (OS Build 14393.82)
Steps / commands required to reproduce the error
see above
Strace of the failing command
Required packages and commands to install
tmux, and/or download latest tmux [2.2] and build from source

This patch to osdep-linux.c will fix it, and should clearly reveal the problem:

char *
osdep_get_cwd(int fd)
{
        static char      target[MAXPATHLEN + 1];
        char            *path;
        pid_t            pgrp, sid;
        ssize_t          n;

#define WIN10_PATCH
#ifdef WIN10_PATCH
        {
            const char      *ttypath;
            ssize_t          n;
            int              retval, ttyfd;

            if ((ttypath = ptsname(fd)) == NULL)
                    return (NULL);
            if ((ttyfd = open(ttypath, O_RDONLY|O_NOCTTY)) == -1)
                    return (NULL);

            fd = ttyfd;
        }
#endif
        if ((pgrp = tcgetpgrp(fd)) == 0)
                return (NULL);

        xasprintf(&path, "/proc/%lld/cwd", (long long) pgrp);
        n = readlink(path, target, MAXPATHLEN);
        free(path);

        if (n == -1 && ioctl(fd, TIOCGSID, &sid) != -1) {
                xasprintf(&path, "/proc/%lld/cwd", (long long) sid);
                n = readlink(path, target, MAXPATHLEN);
                free(path);
        }

        if (n > 0) {
                target[n] = '\0';
                return (target);
        }
        return (NULL);
}

The text was updated successfully, but these errors were encountered:

crzidea · 2016-09-25T08:23:41Z

+1 for this. I am tracking the exactly the same issue all the day. Now I am sure it is an issue of WSL.

aspyrx · 2017-01-10T01:10:41Z

+1, still broken with tmux (from git, tag 2.3) on Ubuntu 16.04.1 LTS on Windows Version 1607 (OS Build 14986.1001).

The following patch works for me:

diff --git a/osdep-linux.c b/osdep-linux.c
index 42712de..6b31931 100644
--- a/osdep-linux.c
+++ b/osdep-linux.c
@@ -20,6 +20,12 @@
 #include <sys/stat.h>
 #include <sys/param.h>

+// https://github.com/Microsoft/BashOnWindows/issues/1063
+#define WIN10_PATCH
+#ifdef WIN10_PATCH
+#include <fcntl.h>
+#endif
+
 #include <event.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -69,9 +75,29 @@ osdep_get_cwd(int fd)
        pid_t            pgrp, sid;
        ssize_t          n;

+// https://github.com/Microsoft/BashOnWindows/issues/1063
+#ifdef WIN10_PATCH
+       char            ttypath[MAXPATHLEN + 1];
+       int                     ttyfd;
+
+       if (ptsname_r(fd, ttypath, sizeof(ttypath))) {
+               return (NULL);
+       }
+
+       if ((ttyfd = open(ttypath, O_RDONLY|O_NOCTTY)) == -1) {
+               return (NULL);
+       }
+
+       fd = ttyfd;
+#endif
+
        if ((pgrp = tcgetpgrp(fd)) == -1)
                return (NULL);

+#ifdef WIN10_PATCH
+       close(ttyfd);
+#endif
+
        xasprintf(&path, "/proc/%lld/cwd", (long long) pgrp);
        n = readlink(path, target, MAXPATHLEN);
        free(path);

sunilmut · 2017-01-10T01:54:04Z

Can someone collect a strace of the failing command on WSL and share it out? There does not seem to be a syscall called 'tcgetpgrp`

strace -o <strace_file> -ff <failing_cmd>

nessss · 2017-03-29T17:38:44Z

Thanks @aspyrx, that patch fixed #{pane_current_path} for me as well. In addition, modifying osdep_get_name in the same way as osdep_get_cwd fixed another issue I was having, namely #{pane_current_command} always returning null.

Here's the updated patch:

diff --git a/osdep-linux.c b/osdep-linux.c
index 42712de..943490a 100644
--- a/osdep-linux.c
+++ b/osdep-linux.c
@@ -20,6 +20,12 @@
 #include <sys/stat.h>
 #include <sys/param.h>

+// https://github.com/Microsoft/BashOnWindows/issues/1063
+#define WIN10_PATCH
+#ifdef WIN10_PATCH
+#include <fcntl.h>
+#endif
+
 #include <event.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -36,9 +42,29 @@ osdep_get_name(int fd, __unused char *tty)
        int      ch;
        pid_t    pgrp;

+// https://github.com/Microsoft/BashOnWindows/issues/1063
+#ifdef WIN10_PATCH
+    char            ttypath[MAXPATHLEN + 1];
+    int                     ttyfd;
+
+    if (ptsname_r(fd, ttypath, sizeof(ttypath))) {
+        return (NULL);
+    }
+
+    if ((ttyfd = open(ttypath, O_RDONLY|O_NOCTTY)) == -1) {
+        return (NULL);
+    }
+
+    fd = ttyfd;
+#endif
+
        if ((pgrp = tcgetpgrp(fd)) == -1)
                return (NULL);

+#ifdef WIN10_PATCH
+    close(ttyfd);
+#endif
+
        xasprintf(&path, "/proc/%lld/cmdline", (long long) pgrp);
        if ((f = fopen(path, "r")) == NULL) {
                free(path);
@@ -69,9 +95,29 @@ osdep_get_cwd(int fd)
        pid_t            pgrp, sid;
        ssize_t          n;

+// https://github.com/Microsoft/BashOnWindows/issues/1063
+#ifdef WIN10_PATCH
+    char            ttypath[MAXPATHLEN + 1];
+    int                     ttyfd;
+
+    if (ptsname_r(fd, ttypath, sizeof(ttypath))) {
+        return (NULL);
+    }
+
+    if ((ttyfd = open(ttypath, O_RDONLY|O_NOCTTY)) == -1) {
+        return (NULL);
+    }
+
+    fd = ttyfd;
+#endif
+
        if ((pgrp = tcgetpgrp(fd)) == -1)
                return (NULL);

+#ifdef WIN10_PATCH
+       close(ttyfd);
+#endif
+
        xasprintf(&path, "/proc/%lld/cwd", (long long) pgrp);
        n = readlink(path, target, MAXPATHLEN);
        free(path);

mmarchini · 2017-05-19T23:17:11Z

@sunilmut here is the full stace file

The failing call to tcgetpgrp happens on line 41252:

[pid  3667] ioctl(16, TIOCGPGRP, 0x7fffc5c1409c) = -1 EINVAL (Invalid argument)

According to this, that call is equivalent to: *argp = tcgetpgrp(16), where argp points to 0x7fffc5c1409c.

And as this call returned -1, it fails again on line 41282 and then fall back to chdir("/") on line 41283:

[pid  3844] fchdir(-1)                  = -1 EBADF (Bad file descriptor)
[pid  3844] chdir("/")                  = 0

If you need more information I'm glad to help.

gpakosz · 2017-06-18T14:24:21Z

Reproduced with tmux HEAD

sunilmut · 2017-06-22T00:27:21Z

@mmarchini - Thanks for the trace and the analysis, and sorry for the delayed response. I have marked it as a bug, which should automatically put it in our backlog of bugs.

gpakosz · 2017-06-22T07:17:42Z

Yep too bad it went untagged for so long, could have been in creators update since a lot of focus was given to tmux etc

benhillis · 2017-08-03T16:48:43Z

I believe this should be fixed in 16257.

Victor-Savu · 2017-08-04T07:20:38Z

@benhillis could you provide a link to the PR you are referring to please? I'd really love to see this fix :)

benhillis · 2017-08-04T14:20:46Z

Unfortunately it's on our internal Microsoft VSTS so I can't share it

Victor-Savu · 2017-08-05T10:19:21Z

Oh, I see. Thanks anyway! I suppose the PR number from you comment will not match the PR number from GitHub so it will be a bit confusing for future readers.

benhillis · 2017-08-05T14:54:46Z

That number is a Windows build number not PR number.

gpakosz · 2017-08-07T16:16:02Z

@benhillis Just tried 16257.rs3_release.170728-1700 and I confirm tmux display -p '#{pane_current_path}' works now.

benhillis · 2017-08-07T16:25:19Z

Awesome, thanks for confirming.

williscool · 2017-11-06T05:56:50Z

How / when do I get the version of windows this is fixed in?

Brian-Perkins · 2017-11-06T17:04:06Z

This fix is available in the Fall Creators Update: https://binged.it/2zBdP2f

whorfin mentioned this issue Jan 23, 2017

Upgrade Trusty to Xenial #482

Closed

gpakosz mentioned this issue Jun 18, 2017

Pane_current_path not working on windows 10 subsystem Linux bash tmux/tmux#762

Closed

gpakosz mentioned this issue Jun 18, 2017

tmux does not pick up on current command running in pane #1412

Closed

sunilmut added the bug label Jun 22, 2017

benhillis added the fixedininsiderbuilds label Aug 3, 2017

benhillis closed this as completed Aug 7, 2017

This was referenced Nov 13, 2017

Tmux Crashes - All attempts to reconnect freeze #2659

Closed

set -m in SH Script fails - Cannot set tty process group (Operation not permitted) #2051

Closed

therealkenc added fixedin1803 and removed bug fixedininsiderbuilds labels Oct 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tcgetpgrp() behavior and tmux #1063

tcgetpgrp() behavior and tmux #1063

whorfin commented Sep 7, 2016 •

edited

Loading

crzidea commented Sep 25, 2016

aspyrx commented Jan 10, 2017

sunilmut commented Jan 10, 2017

nessss commented Mar 29, 2017 •

edited

Loading

mmarchini commented May 19, 2017 •

edited

Loading

gpakosz commented Jun 18, 2017

sunilmut commented Jun 22, 2017

gpakosz commented Jun 22, 2017

benhillis commented Aug 3, 2017

Victor-Savu commented Aug 4, 2017

benhillis commented Aug 4, 2017

Victor-Savu commented Aug 5, 2017

benhillis commented Aug 5, 2017

gpakosz commented Aug 7, 2017 •

edited

Loading

benhillis commented Aug 7, 2017

williscool commented Nov 6, 2017

Brian-Perkins commented Nov 6, 2017

tcgetpgrp() behavior and tmux #1063

tcgetpgrp() behavior and tmux #1063

Comments

whorfin commented Sep 7, 2016 • edited Loading

crzidea commented Sep 25, 2016

aspyrx commented Jan 10, 2017

sunilmut commented Jan 10, 2017

nessss commented Mar 29, 2017 • edited Loading

mmarchini commented May 19, 2017 • edited Loading

gpakosz commented Jun 18, 2017

sunilmut commented Jun 22, 2017

gpakosz commented Jun 22, 2017

benhillis commented Aug 3, 2017

Victor-Savu commented Aug 4, 2017

benhillis commented Aug 4, 2017

Victor-Savu commented Aug 5, 2017

benhillis commented Aug 5, 2017

gpakosz commented Aug 7, 2017 • edited Loading

benhillis commented Aug 7, 2017

williscool commented Nov 6, 2017

Brian-Perkins commented Nov 6, 2017

whorfin commented Sep 7, 2016 •

edited

Loading

nessss commented Mar 29, 2017 •

edited

Loading

mmarchini commented May 19, 2017 •

edited

Loading

gpakosz commented Aug 7, 2017 •

edited

Loading