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:
> Corinna Vinschen wrote:
>> Actually, what I told above is nonsense anyway.  What I saw was the
>> flickering cmd window from a urxvt-X startup without run.  With run it
>> doesn't start at all for me on W7, unless I start it from an existing
>> console window.  And in that case it doesn't matter if the
>> CREATE_NO_WINDOW flag is given or not, it always takes 100% CPU.  So,
>> scratch that, it's no solution at all.
> 
> Rats.  So much for a "good enough" workaround.

OK, the attached patch (on top of run-1.1.11) seems to correct the
problem in my testing.

For testing, I:
  1) used the W7 "workaround" code, even though I was using Vista.
  2) This caused 100% CPU
  3) then, adding the attached patch, I tried again (with the W7
"workaround" code still enabled) and did NOT see 100% CPU.

So, I'm hopeful that this fixes the problem for real, on W7.  However,
with one exception, the production version of 1.1.12 for <= WinXP will
continue to use the "old" code.  That exception is:

for WinXP and above, I forcibly enabled the used "pipes" -- that is,
explicitly creating new Windows Handles and setting them up as the
child's stdio.  The reason for this is, even the "old" run code would
hit 100% CPU in the following scenario, on WinXP:

   1) from a bash prompt in a cmd window
   2) run urxvt-X.exe -display 127.0.0.1:0.0

gave 100% CPU, although launching urxvt-X from a shortcut using the same
invocation did not do so.  With this small change, that problem is
corrected, too.

Corinna, can you give this a try? (I'm happy to send you a compiled
version of the modified run.exe if that helps).

--
Chuck

diff --git a/src/run.c b/src/run.c
index 171f5d7..71a678b 100644
--- a/src/run.c
+++ b/src/run.c
@@ -274,7 +274,7 @@ BOOL setup_invisible_console()
 
 /* returns FALSE only on error conditions (not impl) */
 BOOL configure_startupinfo(STARTUPINFO* psi, BOOL bHaveInvisConsole,
-                           BOOL *bUsingPipes,
+                           BOOL bForceUsingPipes, BOOL *bUsingPipes,
                            HANDLE* hpToChild,  HANDLE* hpFromChild,
                            HANDLE* hpToParent, HANDLE* hpFromParent)
 {
@@ -298,7 +298,7 @@ BOOL configure_startupinfo(STARTUPINFO* psi, BOOL bHaveInvisConsole,
      * should be removed.
      */
 /*
-    if (foo())
+    if (!bForceUsingPipes && foo())
     {
        *bUsingPipes = FALSE;
        return TRUE;
@@ -308,7 +308,7 @@ BOOL configure_startupinfo(STARTUPINFO* psi, BOOL bHaveInvisConsole,
     /* but for now, the only way we KNOW we have a console is
      * if we created it ourselves
      */
-    if (bHaveInvisConsole)
+    if (!bForceUsingPipes && bHaveInvisConsole)
     {
        *bUsingPipes = FALSE;
        return TRUE;
@@ -350,18 +350,20 @@ int start_child(char* cmdline, int wait_for_child)
    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;
 
    setup_win_environ();
 
-#if (defined (__CYGWIN__) && !HAVE_DECL_CYGWIN_CONV_PATH) || defined(__MINGW32__)
-   /* mingw or cygwin-1.5: work around bug in Windows 7, but also
-    * employ on XP and above. This means that setup_invisible_console()
-    * is now used only on <= Win2k */
-   if (os_version >= 0x0501)
+   /* 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)
      {
        HMODULE lib = GetModuleHandle ("kernel32.dll");
        AttachConsoleFP = (BOOL WINAPI (*)(DWORD))
@@ -370,24 +372,33 @@ int start_child(char* cmdline, int wait_for_child)
            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();
-#elif defined (__CYGWIN__) && HAVE_DECL_CYGWIN_CONV_PATH
-   /* cygwin-1.7 */
-   bHaveInvisConsole = TRUE;
 #else
-   /* mingw or cygwin-1.5: work around bug in Windows 7, but also
-    * employ on XP and above. This means that setup_invisible_console()
-    * is now used only on <= Win2k */
-   bHaveInvisConsole = os_version >= 0x0501 ? TRUE : setup_invisible_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.
+    */
+   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);
 #endif
 
    if (!configure_startupinfo(&start, bHaveInvisConsole,
-                              &bUsingPipes,
+                              bForceUsingPipes, &bUsingPipes,
                               &hToChild, &hFromChild,
                               &hToParent, &hFromParent))
       error("could not start %s",cmdline);
@@ -403,11 +414,7 @@ int start_child(char* cmdline, int wait_for_child)
        NULL,    /* process security attributes         */
        NULL,    /* primary thread security attributes  */
        TRUE,    /* handles are inherited,              */
-#if defined(__CYGWIN__) && HAVE_DECL_CYGWIN_CONV_PATH
-       CREATE_NO_WINDOW,
-#else
-       0,       /* creation flags                      */
-#endif
+       creationFlags, /* creation flags                */
        NULL,    /* use parent's environment            */
        NULL,    /* use parent's current directory      */
        &start,  /* STARTUPINFO pointer                 */
@@ -448,17 +455,18 @@ int start_child(char* cmdline, int wait_for_child)
           }
           GetExitCodeProcess (child.hProcess, &retval);
       }
-#if (defined(__CYGWIN__) && !HAVE_DECL_CYGWIN_CONV_PATH) || defined(__MINGW32__)
-     /* mingw or cygwin-1.5: work around bug in Windows 7, but also
-      * employ on XP and above. This means that setup_invisible_console()
-      * is now used only on <= Win2k */
-      if (os_version >= 0x0501)
+
+      /* 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);
       }
-#endif
+
       CloseHandle (child.hThread);
       CloseHandle (child.hProcess);
       if (bUsingPipes)
--
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]