delorie.com/archives/browse.cgi   search  
Mail Archives: cygwin/2006/05/21/15:37:07

X-Spam-Check-By: sourceware.org
Message-ID: <4470C100.6060900@cwilson.fastmail.fm>
Date: Sun, 21 May 2006 15:35:28 -0400
From: Charles Wilson <cygwin AT cwilson DOT fastmail DOT fm>
User-Agent: Thunderbird 1.5.0.2 (Windows/20060308)
MIME-Version: 1.0
To: cygwin AT cygwin DOT com
Subject: Re: [RFA] patch for run.exe -- ATTN: ago
References: <446C1803 DOT 2050901 AT cwilson DOT fastmail DOT fm> <1147949399 DOT 27157 DOT 38 DOT camel AT zipoli DOT prudsys DOT com> <446D84F0 DOT 80609 AT cwilson DOT fastmail DOT fm> <1148221256 DOT 6003 DOT 1 DOT camel AT lupus DOT ago DOT vpn>
In-Reply-To: <1148221256.6003.1.camel@lupus.ago.vpn>
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
List-Subscribe: <mailto:cygwin-subscribe AT cygwin DOT com>
List-Archive: <http://sourceware.org/ml/cygwin/>
List-Post: <mailto:cygwin AT cygwin DOT com>
List-Help: <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Delivered-To: mailing list cygwin AT cygwin DOT com

--------------060601080301040506060908
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit


The attached patch seems to solve all the issues revealed so far.  There 
is one open question, which may dictate a later change but the current 
patch should work as desired on all platforms, and is suitable for 
run-1.1.10 right now, IMO.

Here's the open question: I can find no reliable mechanism for 
determining that a given HANDLE returned by GetStdHandle *actually* 
refers to a true win32 cmd.exe console.  Doing things like calling 
CreateFile() on "CONIN$" and comparing the values doesn't work, and we 
don't necessarily want to do that anyway: if CONIN$ is closed, we don't 
want to reopen it.  Calling functions that *ought* only to work if the 
passed HANDLE is a console IO handle don't work either -- these 
functions succeed when (in my naivete) I believe they should fail!  So, 
I punted.  If we DO find a workable mechanism -- and for some reason we 
decide we care[*] -- there are two simple code changes needed to 
incorporate that future knowledge, and those are detailed in the 
embedded comments.

[*] Really, why should run.exe try to maintain a connection to a calling 
console?  (a) it has no --help output (and if it did, it would probably 
use the message() function which formats text in a windows popup anyway) 
(b) it never tries to output or input anything from the console itself 
(c) the whole point is to hide whatever its client might try to input or 
output from a console.  Actually, I think calling FreeConsole() right 
off the bat is the *right* thing for run.exe to do...unless it is shown 
to cause a problem in some corner case I haven't yet discovered.

So, the attached patch unconditionally and in all cases calls 
FreeConsole() to dissociate from any existing one.  Then, it tries to 
create an invisible console.  If that fails, then it falls back to using 
pipes as ago suggested, only I've pulled that code into a different 
function and modified it a bit.  (You can force even NT/2K/XP to use the 
pipes by compiling with -DDEBUG_FORCE_PIPES).

In either mode -- invisible console or using pipes -- rxvt-unicde-X is 
happy, whether run.exe is invoked via bash-in-rxvt, via bash-in-cmd, or 
via a shortcut, and whether -wait is used or not.

(I also converted C++ comments to C).

--
Chuck


--------------060601080301040506060908
Content-Type: text/plain;
 name="run-combine-invisconsole-and-pipes.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="run-combine-invisconsole-and-pipes.patch"

diff -urN -x .build -x .inst -x .sinst -x .buildlogs run-1.1.9-orig/src/run.c run-1.1.9/src/run.c
--- run-1.1.9-orig/src/run.c	2006-04-06 14:10:55.000000000 -0400
+++ run-1.1.9/src/run.c	2006-05-21 15:21:34.546875000 -0400
@@ -32,6 +32,7 @@
 #define WIN32
 
 #include <windows.h>
+#include <winuser.h>
 #include <string.h>
 #include <malloc.h>
 #include <stdlib.h>
@@ -161,7 +162,7 @@
    xemacs_special(exec);
    ret_code = start_child(cmdline2,wait_for_child);
    if (compact_invocation)
-      for (i = 1; i < argc; i++) // argv[0] was not malloc'ed
+      for (i = 1; i < argc; i++) /* argv[0] was not malloc'ed */
          free(argv[i]);
    else
       for (i = 0; i < argc; i++)
@@ -193,42 +194,240 @@
         free(var);
     }
 }
+BOOL setup_invisible_console()
+{
+   HWINSTA h, horig;
+   USEROBJECTFLAGS oi;
+   DWORD len;
+   BOOL b = FALSE; 
+   HMODULE lib = NULL;
+   HWINSTA WINAPI (*GetProcessWindowStationFP)(void) = NULL;
+   BOOL WINAPI (*GetUserObjectInformationFP)(HANDLE, int, PVOID, DWORD, PDWORD) = NULL;
+   HWINSTA WINAPI (*CreateWindowStationFP)(LPCWSTR, DWORD, DWORD, LPSECURITY_ATTRIBUTES) = NULL;
+   BOOL WINAPI (*SetProcessWindowStationFP)(HWINSTA) = NULL;
+   BOOL WINAPI (*CloseWindowStationFP)(HWINSTA) = NULL;
+
+   /* until we have a mechanism of determining whether a given HANDLE 
+    * returned by GetStdHandles actually derives from a console, 
+    * unconditionally call FreeConsole() on all OSes under all conditions.
+    * See comments in configure_startupinfo(). 
+    */
+   FreeConsole();
 
+   /* First, set up function pointers */
+   if (lib = LoadLibrary ("user32.dll"))
+   {
+       GetProcessWindowStationFP = (HWINSTA WINAPI (*)(void))
+           GetProcAddress (lib, "GetProcessWindowStation");
+       GetUserObjectInformationFP = (BOOL WINAPI (*)(HANDLE, int, PVOID, DWORD, PDWORD))
+           GetProcAddress (lib, "GetUserObjectInformationW"); /* ugly! */
+       CreateWindowStationFP = (HWINSTA WINAPI (*)(LPCWSTR, DWORD, DWORD, LPSECURITY_ATTRIBUTES))
+           GetProcAddress (lib, "CreateWindowStationW"); /* ugly */
+       SetProcessWindowStationFP = (BOOL WINAPI (*)(HWINSTA))
+           GetProcAddress (lib, "SetProcessWindowStation");
+       CloseWindowStationFP = (BOOL WINAPI (*)(HWINSTA))
+           GetProcAddress (lib, "CloseWindowStation");
+
+       if (GetProcessWindowStationFP && 
+            GetUserObjectInformationFP &&
+            CreateWindowStationFP &&
+            SetProcessWindowStationFP &&
+            CloseWindowStationFP)
+       {
+           /* Then, do the work */
+           FreeConsole();
+           h = horig = (*GetProcessWindowStationFP)();
+           if (!horig 
+               || !(*GetUserObjectInformationFP) (horig, UOI_FLAGS, &oi, sizeof (oi), &len )
+               || !(oi.dwFlags & WSF_VISIBLE))
+           {
+               b = AllocConsole();
+           }
+           else
+           {
+               h = (*CreateWindowStationFP) (NULL, 0, STANDARD_RIGHTS_READ, NULL);
+               if (h)
+               {
+                   b = (*SetProcessWindowStationFP) (h);
+               }
+               b = AllocConsole();
+               if (horig && h && h != horig && (*SetProcessWindowStationFP) (horig))
+               {
+                   (*CloseWindowStationFP) (h);
+               }
+           }
+           return b;
+       }
+   }
+   /* otherwise, fail */ 
+   return FALSE;
+}
+
+/* returns FALSE only on error conditions (not impl) */
+BOOL configure_startupinfo(STARTUPINFO* psi, BOOL bHaveInvisConsole,
+                           BOOL *bUsingPipes,
+                           HANDLE* hpToChild,  HANDLE* hpFromChild,
+                           HANDLE* hpToParent, HANDLE* hpFromParent)
+{
+    CONSOLE_CURSOR_INFO cci;
+    SECURITY_ATTRIBUTES handle_attrs;
+    HANDLE hpStdInput[2];
+    HANDLE hpStdOutput[2];
+
+    ZeroMemory (psi, sizeof (STARTUPINFO));
+    psi->cb = sizeof (STARTUPINFO);
+    psi->hStdInput   = GetStdHandle(STD_INPUT_HANDLE);
+    psi->hStdOutput  = GetStdHandle(STD_OUTPUT_HANDLE);
+    psi->hStdError   = GetStdHandle(STD_ERROR_HANDLE);
+    psi->dwFlags     = STARTF_USESHOWWINDOW | STARTF_USESTDHANDLES;
+    psi->wShowWindow = SW_HIDE;
+
+    /* foo() is some magic mechanism for determining that the HANDLEs 
+     * returned by GetStdHandle() are from a console, and not redirected
+     * or ptys of some sort.  If we have such a mechanism, then the 
+     * unconditional FreeConsole() at the top of setup_invisible_console()
+     * should be removed.
+     */
+/*
+    if (foo())
+    {
+       *bUsingPipes = FALSE;
+       return TRUE;
+    }
+*/
+
+    /* but for now, the only way we KNOW we have a console is
+     * if we created it ourselves
+     */
+    if (bHaveInvisConsole)
+    {
+       *bUsingPipes = FALSE;
+       return TRUE;
+    }
+
+    /* otherwise, set up pipes */
+    *bUsingPipes = TRUE;
+
+    handle_attrs.nLength = sizeof(SECURITY_ATTRIBUTES);
+    handle_attrs.bInheritHandle = TRUE;
+    handle_attrs.lpSecurityDescriptor = NULL;
+ 
+    /* create a pipe for child's stdin.  Don't allow child to */
+    /* inherit the write end of the pipe.                     */
+    CreatePipe (&hpStdInput[0], &hpStdInput[1], &handle_attrs, 0);
+    SetHandleInformation ( hpStdInput[1], HANDLE_FLAG_INHERIT, 0);
+ 
+    /* create a pipe for child's stdout.  Don't allow child to */
+    /* inherit the read end of the pipe.                       */
+    CreatePipe (&hpStdOutput[0], &hpStdOutput[1], &handle_attrs, 0);
+    SetHandleInformation ( hpStdOutput[0], HANDLE_FLAG_INHERIT, 0);
+
+    psi->hStdInput   = hpStdInput[0];
+    psi->hStdOutput  = hpStdOutput[1];
+    psi->hStdError   = hpStdOutput[1];
+
+    *hpToChild   = hpStdInput[1];
+    *hpFromChild = hpStdOutput[0];
+    *hpToParent  = hpStdOutput[1];
+    *hpFromParent= hpStdInput[0];
+
+    return TRUE;
+}
 int start_child(char* cmdline, int wait_for_child)
 {
    STARTUPINFO start;
-   SECURITY_ATTRIBUTES sec_attrs;
-   SECURITY_DESCRIPTOR sec_desc;
    PROCESS_INFORMATION child;
    DWORD retval;
+   BOOL bFuncReturn;
+   BOOL bHaveInvisConsole;
+   BOOL bUsingPipes;
+   HANDLE hToChild, hFromChild;
+   HANDLE hToParent, hFromParent;
 
    setup_win_environ();
 
-   memset (&start, 0, sizeof (start));
-   start.cb = sizeof (start);
-   start.dwFlags = STARTF_USESHOWWINDOW | STARTF_USESTDHANDLES;
-   start.wShowWindow = SW_HIDE;
-   start.hStdInput = GetStdHandle(STD_INPUT_HANDLE);
-   start.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
-   start.hStdError = GetStdHandle(STD_ERROR_HANDLE);
-      
-   sec_attrs.nLength = sizeof (sec_attrs);
-   sec_attrs.lpSecurityDescriptor = NULL;
-   sec_attrs.bInheritHandle = FALSE;
+#ifdef DEBUG_FORCE_PIPES
+   bHaveInvisConsole = FALSE;
+   FreeConsole();
+#else
+   bHaveInvisConsole = setup_invisible_console();
+#endif
+
+   if (!configure_startupinfo(&start, bHaveInvisConsole,
+                              &bUsingPipes,
+                              &hToChild, &hFromChild, 
+                              &hToParent, &hFromParent))
+      error("could not start %s",cmdline);
+
+#ifdef DEBUG
+   message("Using Pipes: %s", (bUsingPipes ? "yes" : "no"));
+#endif
+
+   ZeroMemory( &child, sizeof(PROCESS_INFORMATION) );
 
-   if (CreateProcess (NULL, cmdline, &sec_attrs, NULL, TRUE, 0,
-                      NULL, NULL, &start, &child))
+   bFuncReturn = CreateProcess (NULL,
+       cmdline, /* command line                        */
+       NULL,    /* process security attributes         */
+       NULL,    /* primary thread security attributes  */
+       TRUE,    /* handles are inherited,              */
+       0,       /* creation flags                      */
+       NULL,    /* use parent's environment            */
+       NULL,    /* use parent's current directory      */
+       &start,  /* STARTUPINFO pointer                 */
+       &child); /* receives PROCESS_INFORMATION        */
+
+   if (bUsingPipes)
+   {
+       CloseHandle(hFromParent);
+       CloseHandle(hToParent);
+   }
+
+   if (bFuncReturn)
    {
       if (wait_for_child)
       {
-         WaitForSingleObject (child.hProcess, INFINITE);
-         GetExitCodeProcess (child.hProcess, &retval);
-      }
+          if (bUsingPipes)
+          {
+              HANDLE handles[2] = { child.hProcess, hFromChild };
+              COMMTIMEOUTS timeouts;
+    
+              /* only read bytes that are ready, and return immediately */
+              GetCommTimeouts(hFromChild, &timeouts);
+              timeouts.ReadIntervalTimeout = MAXDWORD;
+              timeouts.ReadTotalTimeoutMultiplier = 0;
+              timeouts.ReadTotalTimeoutConstant = 0;
+              SetCommTimeouts(hFromChild, &timeouts);
+    
+              while (WaitForMultipleObjects (2, handles, FALSE, INFINITE) == WAIT_OBJECT_0 + 1)
+              {
+                  char buffer[1024];
+                  DWORD dwRead;
+                  ReadFile (hFromChild, buffer, 1024, &dwRead, NULL);
+              }
+          }
+          else
+          {
+              WaitForSingleObject (child.hProcess, INFINITE);
+          }
+          GetExitCodeProcess (child.hProcess, &retval);
+      } 
       CloseHandle (child.hThread);
       CloseHandle (child.hProcess);
+      if (bUsingPipes)
+      {
+          CloseHandle (hFromChild);
+          CloseHandle (hToChild);
+      }
    }
    else
+   {
+      if (bUsingPipes)
+      {
+          CloseHandle (hFromChild);
+          CloseHandle (hToChild);
+      }
       error("could not start %s",cmdline);
+   }
    return retval;
 }
 void xemacs_special(char* exec)
@@ -353,7 +552,7 @@
     * STARTS WITH x:\ or x:/
     * execpath NOT used
     */
-    else if ((strlen(execname) > 3) && // avoid boundary errors
+    else if ((strlen(execname) > 3) && /* avoid boundary errors */
        (execname[1] == ':') &&
        ((execname[2] == '\\') || (execname[2] == '/')))
    {
@@ -459,10 +658,10 @@
                error("problem reading symbolic link for %s",exec_tmp);
             else
             {
-                // if realname starts with '/' it's a rootpath 
+                /* if realname starts with '/' it's a rootpath */
                 if (real_name[0] == '/')
                     strcpy(exec_tmp2,real_name);
-                else // otherwise, it's relative to the symlink's location
+                else /* otherwise, it's relative to the symlink's location */
                 {
                    CYGWIN_SPLIT_PATH((sym_link_name,exec_tmp2,dummy));
                    if (!endsWith(exec_tmp2,PATH_SEP_CHAR_STR))
@@ -495,8 +694,8 @@
     return retval;
 }void strip_exe(char* s)
 {
-   if ((strlen(s) > 4) && // long enough to have .exe extension
-       // second part not evaluated (short circuit) if exec_arg too short
+   if ((strlen(s) > 4) && /* long enough to have .exe extension */
+       /* second part not evaluated (short circuit) if exec_arg too short */
        (stricmp(&(s[strlen(s)-4]),".exe") == 0))
       s[strlen(s)-4] = '\0';
 }
@@ -557,12 +756,13 @@
       error("internal error - my own name has no path\n%s",modname);
    tmp_execname = p + 1;
    p[0] = '\0';
-   // if invoked by a name like "runxemacs" then strip off
-   // the "run" and let "xemacs" be execname.
-   // To check for this, make that:
-   //   1) first three chars are "run"
-   //   2) but the string doesn't end there, or start ".exe"
-   // Also, set "compact_invocation" TRUE
+   /* if invoked by a name like "runxemacs" then strip off
+    * the "run" and let "xemacs" be execname.
+    * To check for this, make that:
+    *   1) first three chars are "run"
+    *   2) but the string doesn't end there, or start ".exe"
+    * Also, set "compact_invocation" TRUE
+    */
    if ( ((tmp_execname[0] == 'r') || (tmp_execname[0] == 'R')) &&
         ((tmp_execname[1] == 'u') || (tmp_execname[1] == 'U')) &&
         ((tmp_execname[2] == 'n') || (tmp_execname[2] == 'N')) &&


--------------060601080301040506060908
Content-Type: text/plain; charset=us-ascii

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/
--------------060601080301040506060908--

- Raw text -


  webmaster     delorie software   privacy  
  Copyright © 2019   by DJ Delorie     Updated Jul 2019