should wslbridge2 return child exit status?

[iuliantatarascu]

Now that mintty/wsltty#254 is fixed, should wslbridge2 propagate the backend/child exit status, as suggested in this comment mintty/wsltty#254 (comment)?

This would allow mintty --hold error from wsltty to close the window upon a successful command, and hold the window open if the command returns non-zero exit code.

I tried implementing it like this:

diff, CLICK ME

diff --git a/src/wslbridge2-backend.cpp b/src/wslbridge2-backend.cpp
index ad60a1f..13dceae 100644
--- a/src/wslbridge2-backend.cpp
+++ b/src/wslbridge2-backend.cpp
@@ -306,6 +306,7 @@ int main(int argc, char *argv[])
     act.sa_sigaction = [](int x, siginfo_t *y, void *z) { wait(NULL); };
     sigaction(SIGCHLD, &act, NULL);
 
+    int childExitStatus = -1;
     int mfd;
     char ptyname[16];
     const pid_t child = forkpty(&mfd, ptyname, NULL, &winp);
@@ -372,6 +373,18 @@ int main(int argc, char *argv[])
             /* Shutdown I/O sockets when child process terminates */
             if (fds[2].revents & (POLLERR | POLLHUP))
             {
+                int status;
+                if (wait(&status))
+                {
+                    if (WIFEXITED(status))
+                    {
+                        childExitStatus = WEXITSTATUS(status);
+                        printf("exit-status 0x%x 0x%x\n", childExitStatus, status);
+                    }
+                } else {
+                    perror("wait");
+                }
+
                 for (size_t i = 0; i < ARRAYSIZE(ioSockets.sock); i++)
                     shutdown(ioSockets.sock[i], SHUT_RDWR);
 
@@ -454,7 +467,7 @@ int main(int argc, char *argv[])
          * Do not use exit() because it performs clean-up
          * related to user-mode constructs in the library
          */
-        _exit(0);
+        _exit(-1);
     }
     else
         perror("fork");
@@ -463,5 +476,5 @@ int main(int argc, char *argv[])
     for (size_t i = 0; i < ARRAYSIZE(ioSockets.sock); i++)
         close(ioSockets.sock[i]);
 
-    return 0;
+    return childExitStatus;
 }
diff --git a/src/wslbridge2.cpp b/src/wslbridge2.cpp
index f0b49ae..595a22e 100644
--- a/src/wslbridge2.cpp
+++ b/src/wslbridge2.cpp
@@ -481,7 +481,8 @@ int main(int argc, char *argv[])
     if (!ret)
         fatal("SetHandleInformation: %s", GetErrorMessage(GetLastError()).c_str());
 
-    const auto watchdog = std::thread([&]() {
+    DWORD exitCode = 0;
+    auto watchdog = std::thread([&]() {
         WaitForSingleObject(pi.hProcess, INFINITE);
 
         std::vector<char> outVec = readAllFromHandle(outputPipe.rh);
@@ -501,6 +502,10 @@ int main(int argc, char *argv[])
             msg.append(err);
         }
 
+        if (!GetExitCodeProcess(pi.hProcess, &exitCode)) {
+            exitCode = -1;
+        }
+
         /* Exit with error if output/error message is not empty. */
         if (!msg.empty())
             termState.fatal("%s", msg.c_str());
@@ -546,6 +551,8 @@ int main(int argc, char *argv[])
     ret = pthread_create(&tidOutput, nullptr, receive_buffer, nullptr);
     assert(ret == 0);
 
+    watchdog.join();
+
     termState.enterRawMode();
 
     /*
@@ -562,5 +569,5 @@ int main(int argc, char *argv[])
     CloseHandle(pi.hProcess);
     CloseHandle(pi.hThread);
     WSACleanup();
-    termState.exitCleanly(0);
+    termState.exitCleanly(exitCode);
 }

But it doesn't seem to always work. Running ./bin/wslbridge2.exe /bin/false should always return 1. The proper exit code from wait() is not always captured, sometimes WEXITSTATUS wrongly gives 0, sometimes it correctly gives the real status code from the child.

wslbridge2 version e4b44dd236c587613e461d56011016292051d05d

[Biswa96]

I have added something for this. Would you like to check it? I have not properly though about the propagation of exit status to frontend.

[iuliantatarascu]

I guess the exit status was unreliable because of a race condition with the previous wait

wslbridge2/src/wslbridge2-backend.cpp

Line 306 in e4b44dd

act.sa_sigaction = [](int x, siginfo_t *y, void *z) { wait(NULL); };

I have the full propagation working in my fork https://github.com/iuliantatarascu/wslbridge2/commit/626aa01765d408d365529c27e552760a886e9d02

I don't know what scenarios to test, to make sure I didn't break other things.

[iuliantatarascu]

It returns the child's actual exit code on successful execution, and -1 if any other error occurs along the way.

I have only tested with WSL1, I don't know about WSL2, things might be different there.

[Biswa96]

BTW, in your same code the exit status is messed up. If GetExitCodeProcess() is added in the watchdog thread then it will capture the return value of wsl.exe process NOT the exit status of child process of wslbridge2-backend. The proper way is to send the exit status through socket. See wslbridge project, parent of this one.