X-Recipient: archive-cygwin AT delorie DOT com
X-Spam-Check-By: sourceware.org
Date: Tue, 9 Mar 2010 12:36:08 +0100
From: Corinna Vinschen <corinna-cygwin AT cygwin DOT com>
To: cygwin AT cygwin DOT com
Subject: Re: Cygwin 1.7: Concurrency Issue with Shared State Initialization
Message-ID: <20100309113608.GH6505@calimero.vinschen.de>
Mail-Followup-To: cygwin AT cygwin DOT com
References: <B10B50309DC54D45A855FD66F56015F71D07C78CAE AT DEWDFECCR03 DOT wdf DOT sap DOT corp>  <20100308195656 DOT GA17237 AT ednor DOT casa DOT cgf DOT cx>  <B10B50309DC54D45A855FD66F56015F71D07C79605 AT DEWDFECCR03 DOT wdf DOT sap DOT corp>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <B10B50309DC54D45A855FD66F56015F71D07C79605@DEWDFECCR03.wdf.sap.corp>
User-Agent: Mutt/1.5.20 (2009-06-14)
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Precedence: bulk
List-Id: <cygwin.cygwin.com>
List-Unsubscribe: <mailto:cygwin-unsubscribe-archive-cygwin=delorie DOT com AT cygwin DOT com>
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

On Mar  9 11:08, Schmidt, Oliver wrote:
> Hi Christopher,
> 
> Fist of all thanks for your immediate feedback :-)
> 
> >>    /* Initialize installation root dir. */
> >>     if (!installation_root[0])
> >>       init_installation_root ();
> 
> > I'll check in something tonight which attempts to solve this problem.
> 
> Thanks in advance.
> 
> > It's a somewhat tricky problem because adding a mutex here would slow
> > down every invocation of a cygwin program and we don't want to add to
> > the "Why is Cygwin so slow???" scenarios if we can help it.
> 
> I understand your concern for sure ;-)
> 
> Maybe that's the very thing you're thinking about but ... AFAIK a
> spinlock is the usual paradigm in scenarios where one doesn't
> anticipate contention but needs to be aware of it 'just in case'. With
> InterlockedCompareExchange() and Sleep() it should be quite simple to
> create one that's very efficient in the usual scenario.

Does the below patch fix this for you?

	* shared.cc (installation_root_init): New DLL-shared variable.
	(inst_root_inited): Remove.
	(init_installation_root): Drop setting inst_root_inited.
	(memory_init): Make inst_root_inited locale variable here.
	Only try to initialize installation_root if parent is a non-Cygwin
	process.  Guard initialization with spinlock.

Index: shared.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/shared.cc,v
retrieving revision 1.130
diff -u -p -r1.130 shared.cc
--- shared.cc	18 Dec 2009 20:32:04 -0000	1.130
+++ shared.cc	9 Mar 2010 11:31:12 -0000
@@ -34,10 +34,10 @@ user_info NO_COPY *user_shared;
 HANDLE NO_COPY cygwin_shared_h;
 HANDLE NO_COPY cygwin_user_h;
 
+LONG installation_root_init __attribute__((section (".cygwin_dll_common"), shared));
 WCHAR installation_root[PATH_MAX] __attribute__((section (".cygwin_dll_common"), shared));
 UNICODE_STRING installation_key __attribute__((section (".cygwin_dll_common"), shared));
 WCHAR installation_key_buf[18] __attribute__((section (".cygwin_dll_common"), shared));
-static bool inst_root_inited;
 
 /* Use absolute path of cygwin1.dll to derive the Win32 dir which
    is our installation_root.  Note that we can't handle Cygwin installation
@@ -115,8 +115,6 @@ init_installation_root ()
       installation_key.Length = 0;
       installation_key.Buffer[0] = L'\0';
     }
-
-  inst_root_inited = true;
 }
 
 /* This function returns a handle to the top-level directory in the global
@@ -412,19 +410,29 @@ shared_info::initialize ()
 void
 memory_init (bool init_cygheap)
 {
+  bool inst_root_inited = false;
+
   getpagesize ();
 
-  /* Initialize the Cygwin heap, if necessary */
+  /* Initialize the Cygwin heap.  This is only necessary if the process is
+     started from a non-Cygwin process. */
   if (init_cygheap)
     {
       cygheap_init ();
       cygheap->user.init ();
+      /* Initialize installation root dir. */
+      LONG init = InterlockedCompareExchange (&installation_root_init, 1L, 0L);
+      if (!init)
+	{
+	  init_installation_root ();
+	  inst_root_inited = true;
+	  InterlockedIncrement (&installation_root_init);
+	}
+      else
+      	while (installation_root_init == 1L)
+	  low_priority_sleep (0);
     }
 
-  /* Initialize installation root dir. */
-  if (!installation_root[0])
-    init_installation_root ();
-
   /* Initialize general shared memory */
   shared_locations sh_cygwin_shared;
   cygwin_shared = (shared_info *) open_shared (L"shared",


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

--
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