This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [ANNOUNCEMENT] Updated: run-1.1.11-1


Charles Wilson wrote:

> If we already HAVE true console handles, then we shouldn't FreeConsole
> and try to recreate our own...

OK, how about this?  Seems to work on Vista(cyg1.5, cyg1.7, mingw) x
gui/cui/bat.

The basic idea is to try and re-use whatever console is available, if
any, when run is started.  If this is the invisible console created by
cygwin as part of process startup, great. If it is inherited from the
calling window, fine.  But never try to explicitly hide the console if
it already exists (that way, you can call 'run foo' from a cmd box
without it disappearing on you).

Also, if creating a new console, then do it in the parent (run) process
and hide it there -- even for W7, by using the message-only trick --
instead of attaching to the child's console and manipulating it after
the fact.

I *think* this slight refactoring will allow us to add any additional
tweaks needed by W7 with only small, localized changes, so I'm hopeful
that this patch can actual be checked in even if it is not perfect.

--
Chuck
Index: src/run.c
===================================================================
RCS file: /cvs/cygwin-apps/run/src/run.c,v
retrieving revision 1.8
diff -u -p -r1.8 run.c
--- src/run.c	16 Aug 2009 03:26:42 -0000	1.8
+++ src/run.c	17 Aug 2009 22:33:23 -0000
@@ -40,11 +40,13 @@
 
 #include <windows.h>
 #include <winuser.h>
+#include <shellapi.h>
 #include <string.h>
 #include <malloc.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdarg.h>
+#include <process.h>
 
 #include "run.h"
 
@@ -63,6 +65,9 @@ WinMainCRTStartup() { mainCRTStartup(); 
 DWORD os_version;
 char buffer[1024];
 
+static BOOL target_is_gui(const char* target_path);
+static void setup_win_environ(void);
+
 int WINAPI
 WinMain (HINSTANCE hSelf, HINSTANCE hPrev, LPSTR cmdline, int nShow)
 {
@@ -165,11 +170,61 @@ WinMain (HINSTANCE hSelf, HINSTANCE hPre
    Trace(("exec\t%s\nexecname\t%s\nexecpath\t%s\n",
          exec,execname,execpath));
 
-   wait_for_child = build_cmdline(cmdline2,exec,argc,argv);
+   wait_for_child = build_cmdline(cmdline2,exec,&argc,argv);
    Trace((cmdline2));
 
    xemacs_special(exec);
-   ret_code = start_child(cmdline2,wait_for_child);
+   if (target_is_gui (exec))
+     {
+       /* much simpler if target is gui, because we don't
+        * actually need to worry about consoles, stdio
+        * handles, etc.  If -wait, then delegate to _spawnv,
+        * since we have the argv array. However, because
+        * _spawnv (_P_NOWAIT) doesn't work reliably on
+        * cygwin, use a lobotomized version of CreateProcess
+        * (but still don't worry about handles or consoles).
+        */
+       setup_win_environ();
+       if (wait_for_child)
+         {
+           Trace(("gui wait for child: %s", exec));
+           /* ret_code is the child exit status for P_WAIT */
+           ret_code = _spawnv (_P_WAIT, exec, argv);
+           if (ret_code < 0)
+             error("could not start %s", exec);
+           return (int)ret_code;
+         }
+       else
+         {
+           STARTUPINFO start;
+           PROCESS_INFORMATION child;
+           ZeroMemory( &child, sizeof(PROCESS_INFORMATION) );
+           ZeroMemory (&start, sizeof (STARTUPINFO));
+           start.cb = sizeof (STARTUPINFO);
+           Trace(("Launch GUI target, async: %s", cmdline2));
+           ret_code = CreateProcess (NULL,
+               cmdline2,/* command line                        */
+               NULL,    /* process security attributes         */
+               NULL,    /* primary thread security attributes  */
+               FALSE,   /* handles are NOT inherited,          */
+               0,       /* creation flags                      */
+               NULL,    /* use parent's environment            */
+               NULL,    /* use parent's current directory      */
+               &start,  /* STARTUPINFO pointer                 */
+               &child); /* receives PROCESS_INFORMATION        */
+           if (ret_code == 0)
+             {
+               Trace(("getlasterror: %d\n", GetLastError()));
+               error("could not start %s", exec);
+             }
+           return 0;
+         }
+     }
+   else
+     {
+       Trace(("Launch non-GUI target"));
+       ret_code = start_child(exec, cmdline2,wait_for_child);
+     }
    if (compact_invocation)
       for (i = 1; i < argc; i++) /* argv[0] was not malloc'ed */
          free(argv[i]);
@@ -179,6 +234,51 @@ WinMain (HINSTANCE hSelf, HINSTANCE hPre
    return (int) ret_code;
 }
 
+static BOOL target_is_gui(const char* target_path)
+{
+  char p, e;
+  DWORD_PTR d =
+  SHGetFileInfoA(target_path,    /* LPCSTR pszPath         */
+                 0,              /* DWORD dwFileAttributes */
+                 NULL,           /* SHFILEINFO *psfi       */
+                 0,              /* UINT cbFileInfo        */
+                 SHGFI_EXETYPE); /* UINT uFlags            */
+
+  p = LOBYTE (LOWORD (d));
+  e = HIBYTE (LOWORD (d));
+  Trace(("p=%c\ne=%c\nv=%d\n", p, e, HIWORD(d)));
+  return ( (((p=='P') || (p=='N')) && (e=='E')) && (HIWORD(d) != 0) );
+}
+
+static BOOL have_console (void)
+{
+  /* Note: we do it this way instead of using GetConsoleWindow()
+   * because we want it to work on < w2k.
+   */
+  HANDLE hConOut;
+  SECURITY_ATTRIBUTES  sa;
+  CONSOLE_SCREEN_BUFFER_INFO buffInfo;
+
+  BOOL retval = FALSE;
+
+  sa.nLength = sizeof(sa);
+  sa.bInheritHandle = TRUE;
+  sa.lpSecurityDescriptor = NULL;
+  hConOut = CreateFile( "CONOUT$", GENERIC_WRITE | GENERIC_READ,
+                        FILE_SHARE_READ | FILE_SHARE_WRITE, &sa,
+                        OPEN_EXISTING, 0, 0 );
+  if (hConOut != INVALID_HANDLE_VALUE)
+    {
+      if (GetConsoleScreenBufferInfo(hConOut,&buffInfo))
+        {
+          retval = TRUE;
+        }
+      CloseHandle (hConOut);
+    }
+  Trace(("have console? %s", (retval ? "yes" : "no")));
+  return retval;
+}
+
 /* Copy cygwin environment variables to the Windows environment if they're not
  * already there. */
 static void setup_win_environ(void)
@@ -203,6 +303,7 @@ static void setup_win_environ(void)
         free(var);
     }
 }
+
 BOOL setup_invisible_console()
 {
    HWINSTA h, horig;
@@ -216,12 +317,14 @@ BOOL setup_invisible_console()
    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();
+   Trace(("Setting up invisible console using separate WindowStation"));
+
+   /* paranoia */
+   if (have_console ())
+     {
+       Trace(("Already have a console; not setting up another one."));
+       return TRUE;
+     }
 
    /* First, set up function pointers */
    if (lib = LoadLibrary ("user32.dll"))
@@ -265,15 +368,17 @@ BOOL setup_invisible_console()
                    (*CloseWindowStationFP) (h);
                }
            }
+           Trace(("Succeeded in setting up console on separate Window Station? %s", (b ? "yes" : "no")));
            return b;
        }
    }
    /* otherwise, fail */
+   Trace(("Failed to set up console on separate Window Station"));
    return FALSE;
 }
 
 /* returns FALSE only on error conditions (not impl) */
-BOOL configure_startupinfo(STARTUPINFO* psi, BOOL bHaveInvisConsole,
+BOOL configure_startupinfo(STARTUPINFO* psi, BOOL bHaveConsole,
                            BOOL bForceUsingPipes, BOOL *bUsingPipes,
                            HANDLE* hpToChild,  HANDLE* hpFromChild,
                            HANDLE* hpToParent, HANDLE* hpFromParent)
@@ -283,6 +388,8 @@ BOOL configure_startupinfo(STARTUPINFO* 
     HANDLE hpStdInput[2];
     HANDLE hpStdOutput[2];
 
+    Trace(("Configuring start info for child process"));
+
     ZeroMemory (psi, sizeof (STARTUPINFO));
     psi->cb = sizeof (STARTUPINFO);
     psi->hStdInput   = GetStdHandle(STD_INPUT_HANDLE);
@@ -291,27 +398,29 @@ BOOL configure_startupinfo(STARTUPINFO* 
     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 we have a console and have not requested pipes,
+     * ensure that the child stdio handles are properly
+     * connected.
      */
-/*
-    if (!bForceUsingPipes && foo())
+    if (!bForceUsingPipes && bHaveConsole)
     {
-       *bUsingPipes = FALSE;
-       return TRUE;
-    }
-*/
+      SECURITY_ATTRIBUTES  sa;
 
-    /* but for now, the only way we KNOW we have a console is
-     * if we created it ourselves
-     */
-    if (!bForceUsingPipes && bHaveInvisConsole)
-    {
-       *bUsingPipes = FALSE;
-       return TRUE;
+      sa.nLength = sizeof(sa);
+      sa.bInheritHandle = TRUE;
+      sa.lpSecurityDescriptor = NULL;
+
+      *bUsingPipes = FALSE;
+      psi->hStdInput   = CreateFile( "CONIN$", GENERIC_WRITE | GENERIC_READ,
+                         FILE_SHARE_READ, &sa,
+                         OPEN_EXISTING, 0, 0 );
+      psi->hStdOutput  = CreateFile( "CONOUT$", GENERIC_WRITE | GENERIC_READ,
+                         FILE_SHARE_WRITE, &sa,
+                         OPEN_EXISTING, 0, 0 );
+      psi->hStdError   = psi->hStdOutput;
+
+      Trace(("Have a console, and not requesting pipes: connecting child stdio to console"));
+      return TRUE;
     }
 
     /* otherwise, set up pipes */
@@ -340,30 +449,32 @@ BOOL configure_startupinfo(STARTUPINFO* 
     *hpToParent  = hpStdOutput[1];
     *hpFromParent= hpStdInput[0];
 
+    Trace(("Set up pipes for child stdio"));
     return TRUE;
 }
-int start_child(char* cmdline, int wait_for_child)
+
+int start_child(char *exec, char* cmdline, int wait_for_child)
 {
    STARTUPINFO start;
    PROCESS_INFORMATION child;
    DWORD retval;
    BOOL bFuncReturn;
-   BOOL bHaveInvisConsole;
    BOOL bUsingPipes;
    BOOL bForceUsingPipes = FALSE;
    HANDLE hToChild, hFromChild;
    HANDLE hToParent, hFromParent;
    BOOL WINAPI (*AttachConsoleFP)(DWORD) = NULL;
    HWND WINAPI (*GetConsoleWindowFP)(VOID) = NULL;
-   DWORD creationFlags = 0;
+   BOOL bHaveConsole = FALSE;
+   BOOL bUseMessageOnlyWorkaround = FALSE;
 
-   setup_win_environ();
+   setup_win_environ ();
+   bHaveConsole = have_console ();
 
-   /* Work around bug in Windows 7. For Vista and below, continue
-    * to use the more reliable setup_invisible_console() and its
-    * separate WindowStation approach; for W7 we need these pointers.
-    */
-   if (os_version >= 0x0601)
+   /* Need this to work around a bug in Windows 7 */
+   bUseMessageOnlyWorkaround = (os_version >= 0x0601);
+
+   if (bUseMessageOnlyWorkaround)
      {
        HMODULE lib = GetModuleHandle ("kernel32.dll");
        AttachConsoleFP = (BOOL WINAPI (*)(DWORD))
@@ -372,49 +483,41 @@ int start_child(char* cmdline, int wait_
            GetProcAddress (lib, "GetConsoleWindow");
        if (!AttachConsoleFP || !GetConsoleWindowFP)
            os_version = 0;
-#if defined(__CYGWIN__) && HAVE_DECL_CYGWIN_CONV_PATH
-       /* and for cygwin-1.7, also this, because cygwin kernel
-        * will create a hidden console -- or attach child to an
-        * existing one -- for us.
-        */
-       /* creationFlags |= CREATE_NO_WINDOW; */
-#endif
      }
 
 #ifdef DEBUG_FORCE_PIPES
-   bHaveInvisConsole = FALSE;
    bForceUsingPipes = TRUE;
    FreeConsole();
+   bHaveConsole = FALSE;
 #else
-   /* Work around bug in Windows 7. For Vista and below, continue
-    * to use the more reliable setup_invisible_console() and its
-    * separate WindowStation approach.
-    */
-   bHaveInvisConsole = os_version >= 0x0601 ? TRUE : setup_invisible_console();
-   /* Fix issue with 100% CPU usage when launching certain apps from
-    * a cmd.exe box
-    */
-   bForceUsingPipes = (os_version >= 0x0501);
+   if (bUseMessageOnlyWorkaround)
+     {
+       if (!bHaveConsole)
+         {
+           AllocConsole ();
+           bHaveConsole = TRUE;
+         }
+       SetParent ((*GetConsoleWindowFP) (), HWND_MESSAGE);
+     }
+   else if (!bHaveConsole)
+     {
+       bHaveConsole = setup_invisible_console();
+     }
 #endif
 
-   if (!configure_startupinfo(&start, bHaveInvisConsole,
+   if (!configure_startupinfo(&start, bHaveConsole,
                               bForceUsingPipes, &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) );
-
    bFuncReturn = CreateProcess (NULL,
        cmdline, /* command line                        */
        NULL,    /* process security attributes         */
        NULL,    /* primary thread security attributes  */
        TRUE,    /* handles are inherited,              */
-       creationFlags, /* creation flags                */
+       0,       /* creation flags                      */
        NULL,    /* use parent's environment            */
        NULL,    /* use parent's current directory      */
        &start,  /* STARTUPINFO pointer                 */
@@ -456,17 +559,6 @@ int start_child(char* cmdline, int wait_
           GetExitCodeProcess (child.hProcess, &retval);
       }
 
-      /* Work around bug in Windows 7. For Vista and below, continue
-       * to use the more reliable setup_invisible_console() and its
-       * separate WindowStation approach.
-       */
-      if (os_version >= 0x0601)
-      {
-        FreeConsole ();
-        (*AttachConsoleFP) (child.dwProcessId);
-        SetParent ((*GetConsoleWindowFP) (), HWND_MESSAGE);
-      }
-
       CloseHandle (child.hThread);
       CloseHandle (child.hProcess);
       if (bUsingPipes)
@@ -486,6 +578,7 @@ int start_child(char* cmdline, int wait_
    }
    return retval;
 }
+
 void xemacs_special(char* exec)
 {
   /*
@@ -518,29 +611,33 @@ void xemacs_special(char* exec)
       }
    }
 }
-int build_cmdline(char* new_cmdline, char* exec, int argc, char* argv[])
+int build_cmdline(char* new_cmdline, char* exec, int *argc, char* argv[])
 {
    int retval = FALSE;
-   int first_arg = 1;
    int i;
    int char_cnt = 0;
    /*
     * look for "-wait" as first true argument; we'll apply that ourselves
     */
-   if ((argc >= 2) && (stricmp(argv[1],"-wait") == 0))
+   if ((*argc >= 2) && (stricmp(argv[1],"-wait") == 0))
    {
       retval = TRUE;
-      first_arg++;
+      /* remove -wait from argv array */
+      free (argv[1]);
+      for (i = 1; i < *argc-1; i++)
+        argv[i] = argv[i+1];
+      argv[*argc-1] = NULL;
+      *argc -= 1;
    }
 
    char_cnt = strlen(exec);
-   for (i = first_arg; i < argc; i++)
+   for (i = 1; i < *argc; i++)
       char_cnt += strlen(argv[i]);
    if (char_cnt > MAX_ARGS*MAX_PATH) /* then we ran out of room */
       error("command line too long -\n%s",new_cmdline);
 
    strcpy(new_cmdline,exec);
-   for (i = first_arg; i < argc; i++)
+   for (i = 1; i < *argc; i++)
    {
       strcat(new_cmdline," ");
       strcat(new_cmdline,argv[i]);
@@ -737,6 +834,7 @@ void process_execname(char *exec, const 
    strcpy (exec, exec_tmp2);
 #endif
 }
+
 int endsWith(const char* s1, const char* s2)
 {
     int len1;
@@ -748,7 +846,9 @@ int endsWith(const char* s1, const char*
         if (stricmp(&(s1[len1-len2]),s2) == 0)
             retval = TRUE;
     return retval;
-}void strip_exe(char* s)
+}
+
+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 */
Index: src/run.h
===================================================================
RCS file: /cvs/cygwin-apps/run/src/run.h,v
retrieving revision 1.2
diff -u -p -r1.2 run.h
--- src/run.h	16 Aug 2009 03:18:48 -0000	1.2
+++ src/run.h	17 Aug 2009 22:33:23 -0000
@@ -74,7 +74,7 @@
 #endif
 
 #define NUM_EXTENSIONS 2
-const char* exts[NUM_EXTENSIONS] = { "", ".exe" };
+const char* exts[NUM_EXTENSIONS] = { ".exe", "" };
 
 char* pfopen(char *retval, const char *name, const char *dirs);
 void error(char* fmt, ...);
@@ -84,9 +84,9 @@ int get_exec_name_and_path(char* execnam
 char* my_strtok(char* s, const char* delim, char** lasts);
 int parse_cmdline_to_arg_array(char* argv[MAX_ARGS], char* cmdline);
 void strip_exe(char* s);
-int start_child(char* cmdline, int wait_for_child);
+int start_child(char *exec, char* cmdline, int wait_for_child);
 void xemacs_special(char* exec);
-int build_cmdline(char* new_cmdline, char* exec, int argc, char* argv[]);
+int build_cmdline(char* new_cmdline, char* exec, int *argc, char* argv[]);
 void process_execname(char *exec, const char* execname, const char* execpath);
 int fileExists(char* fullname, const char* path, const char* name);
 int endsWith(const char* s1, const char* s2);
--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]