[newlib-cygwin] forkables: On fork failure, retry with hardlinks.

Corinna Vinschen corinna@sourceware.org
Fri Feb 8 11:37:00 GMT 2019


https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=ece7282f329fd3616a9a1089aa0f0c26a5c01cc1

commit ece7282f329fd3616a9a1089aa0f0c26a5c01cc1
Author: Michael Haubenwallner <michael.haubenwallner@ssi-schaefer.com>
Date:   Wed Dec 7 11:58:28 2016 +0100

    forkables: On fork failure, retry with hardlinks.
    
    To support in-cygwin package managers, the fork() implementation must
    not rely on .exe and .dll files to stay in their original location, as
    the package manager's job is to replace these files.  Instead, when the
    first fork try fails, and we have NTFS, we use hardlinks to the original
    binaries in /var/run/cygfork/ to create the child process during the
    second fork try, along the main.exe.local file to enable the "DotLocal
    Dll Redirection" feature for the dlls.
    
    The (probably few) users that need an update-safe fork manually have to
    create the /var/run/cygfork/ directory for now, using:
    mkdir --mode=a=rwxt /var/run/cygfork
    
    	* child_info.h: Bump CURR_CHILD_INFO_MAGIC.
    	(enum child_status): Add _CI_SILENTFAIL flag.
    	(struct child_info): Add silentfail setter and getter.
    	* winsup.h (child_copy): Add bool silentfail parameter.
    	* cygheap.cc: Pass silentfail parameter to child_copy.
    	* dcrt0.cc: Ditto.
    	* dll_init.h (struct dll): Define public inline method forkedntname.
    	(struct dll_list): Declare private method find_by_forkedntname.
    	* dll_init.cc (struct dll_list): Implement find_by_forkedntname.
    	(dll_list::alloc): Use find_by_forkedntname when in load after fork.
    	(dll_list::load_after_fork_impl): Load dlls using dll::forkedntname.
    	* fork.cc (frok::parent): Set silentfail child info flag.  Pass
    	silentfail parameter to child_copy.  Use forkedntname of
    	dlls.main_executable.
    	(fork): When first dofork run failed and did not use forkables,
    	run dofork again with_forkables set to true.
    	(child_copy): Use debug_printf if silentfail is true,
    	system_printf otherwise.

Diff:
---
 winsup/cygwin/child_info.h | 13 ++++++++++--
 winsup/cygwin/cygheap.cc   |  3 ++-
 winsup/cygwin/dcrt0.cc     |  4 ++--
 winsup/cygwin/dll_init.cc  | 49 ++++++++++++++++++++++++++++++++--------------
 winsup/cygwin/dll_init.h   |  5 +++++
 winsup/cygwin/fork.cc      | 37 +++++++++++++++++++++++-----------
 winsup/cygwin/sigproc.cc   |  5 ++++-
 winsup/cygwin/winsup.h     |  2 +-
 8 files changed, 85 insertions(+), 33 deletions(-)

diff --git a/winsup/cygwin/child_info.h b/winsup/cygwin/child_info.h
index f7a1441..6726411 100644
--- a/winsup/cygwin/child_info.h
+++ b/winsup/cygwin/child_info.h
@@ -21,7 +21,8 @@ enum child_status
 {
   _CI_STRACED	 = 0x01,
   _CI_ISCYGWIN	 = 0x02,
-  _CI_SAW_CTRL_C = 0x04
+  _CI_SAW_CTRL_C = 0x04,
+  _CI_SILENTFAIL = 0x08
 };
 
 #define OPROC_MAGIC_MASK 0xff00ff00
@@ -36,7 +37,7 @@ enum child_status
 #define EXEC_MAGIC_SIZE sizeof(child_info)
 
 /* Change this value if you get a message indicating that it is out-of-sync. */
-#define CURR_CHILD_INFO_MAGIC 0xc96f5e9U
+#define CURR_CHILD_INFO_MAGIC 0x3ee00652U
 
 #define NPROCS	256
 
@@ -82,6 +83,7 @@ public:
   bool isstraced () const {return !!(flag & _CI_STRACED);}
   bool iscygwin () const {return !!(flag & _CI_ISCYGWIN);}
   bool saw_ctrl_c () const {return !!(flag & _CI_SAW_CTRL_C);}
+  bool silentfail () const {return !!(flag & _CI_SILENTFAIL);}
   void prefork (bool = false);
   void cleanup ();
   void postfork (pinfo& child)
@@ -91,6 +93,13 @@ public:
     child.set_rd_proc_pipe (rd_proc_pipe);
     rd_proc_pipe = NULL;
   }
+  void silentfail (bool f)
+  {
+    if (f)
+      flag |= _CI_SILENTFAIL;
+    else
+      flag &= ~_CI_SILENTFAIL;
+  }
 };
 
 class mount_info;
diff --git a/winsup/cygwin/cygheap.cc b/winsup/cygwin/cygheap.cc
index 0e9741c..eb60cf2 100644
--- a/winsup/cygwin/cygheap.cc
+++ b/winsup/cygwin/cygheap.cc
@@ -78,7 +78,8 @@ cygheap_fixup_in_child (bool execed)
 {
   cygheap_max = cygheap = (init_cygheap *) _cygheap_start;
   _csbrk ((char *) child_proc_info->cygheap_max - (char *) cygheap);
-  child_copy (child_proc_info->parent, false, "cygheap", cygheap, cygheap_max, NULL);
+  child_copy (child_proc_info->parent, false, child_proc_info->silentfail (),
+	      "cygheap", cygheap, cygheap_max, NULL);
   cygheap_init ();
   debug_fixup_after_fork_exec ();
   if (execed)
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index 5cdf01c..78506d4 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -601,7 +601,7 @@ child_info_fork::handle_fork ()
   myself->uid = cygheap->user.real_uid;
   myself->gid = cygheap->user.real_gid;
 
-  child_copy (parent, false,
+  child_copy (parent, false, silentfail (),
 	      "dll data", dll_data_start, dll_data_end,
 	      "dll bss", dll_bss_start, dll_bss_end,
 	      "user heap", cygheap->user_heap.base, cygheap->user_heap.ptr,
@@ -625,7 +625,7 @@ child_info_fork::handle_fork ()
 
   /* step 2 now that the dll has its heap filled in, we can fill in the
      user's data and bss since user_data is now filled out. */
-  child_copy (parent, false,
+  child_copy (parent, false, silentfail (),
 	      "data", user_data->data_start, user_data->data_end,
 	      "bss", user_data->bss_start, user_data->bss_end,
 	      NULL);
diff --git a/winsup/cygwin/dll_init.cc b/winsup/cygwin/dll_init.cc
index 90b3940..82b0656 100644
--- a/winsup/cygwin/dll_init.cc
+++ b/winsup/cygwin/dll_init.cc
@@ -289,6 +289,19 @@ dll_list::find_by_modname (PCWCHAR modname)
   return NULL;
 }
 
+/* Look for a dll based on the ntname used
+   to dynamically reload in forked child. */
+dll *
+dll_list::find_by_forkedntname (PCWCHAR ntname)
+{
+  dll *d = &start;
+  while ((d = d->next) != NULL)
+    if (!wcscasecmp (ntname, d->forkedntname ()))
+      return d;
+
+  return NULL;
+}
+
 #define RETRIES 1000
 
 /* Allocate space for a dll struct. */
@@ -308,8 +321,11 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
   /* Already loaded?  For linked DLLs, only compare the basenames.  Linked
      DLLs are loaded using just the basename and the default DLL search path.
      The Windows loader picks up the first one it finds.
-     This also applies to cygwin1.dll and the main-executable (DLL_SELF).  */
-  dll *d = (type != DLL_LOAD) ? dlls.find_by_modname (modname) : dlls[ntname];
+     This also applies to cygwin1.dll and the main-executable (DLL_SELF).
+     When in_load_after_fork, dynamically loaded dll's are reloaded
+     using their parent's forkable_ntname, if available.  */
+  dll *d = (type != DLL_LOAD) ? dlls.find_by_modname (modname) :
+	   in_load_after_fork ? dlls.find_by_forkedntname (ntname) : dlls[ntname];
   if (d)
     {
       /* We only get here in the forkee. */
@@ -714,14 +730,16 @@ void dll_list::load_after_fork_impl (HANDLE parent, dll* d, int retries)
 	  fabort ("unable to release protective reservation (%p) for %W, %E",
 		  d->handle, d->ntname);
 
-	HMODULE h = LoadLibraryExW (buffered_shortname (d->ntname),
+	HMODULE h = LoadLibraryExW (buffered_shortname (d->forkedntname ()),
 				    NULL, DONT_RESOLVE_DLL_REFERENCES);
 	if (!h)
-	  fabort ("unable to create interim mapping for %W, %E", d->ntname);
+	  fabort ("unable to create interim mapping for %W (using %W), %E",
+		  d->ntname, buffered_shortname (d->forkedntname ()));
 	if (h != d->handle)
 	  {
-	    sigproc_printf ("%W loaded in wrong place: %p != %p",
-			    d->ntname, h, d->handle);
+	    sigproc_printf ("%W (using %W) loaded in wrong place: %p != %p",
+			    d->ntname, buffered_shortname (d->forkedntname ()),
+			    h, d->handle);
 	    FreeLibrary (h);
 	    PVOID reservation = reserve_at (d->ntname, h,
 					    d->handle, d->image_size);
@@ -732,8 +750,8 @@ void dll_list::load_after_fork_impl (HANDLE parent, dll* d, int retries)
 	    if (retries < DLL_RETRY_MAX)
 	      load_after_fork_impl (parent, d, retries+1);
 	    else
-	       fabort ("unable to remap %W to same address as parent (%p) - try running rebaseall",
-		       d->ntname, d->handle);
+	       fabort ("unable to remap %W (using %W) to same address as parent (%p) - try running rebaseall",
+		       d->ntname, buffered_shortname (d->forkedntname ()), d->handle);
 
 	    /* once the above returns all the dlls are mapped; release
 	       the reservation and continue unwinding */
@@ -761,20 +779,21 @@ void dll_list::load_after_fork_impl (HANDLE parent, dll* d, int retries)
 	  /* Free the library using our parent's handle: it's identical
 	     to ours or we wouldn't have gotten this far */
 	  if (!FreeLibrary (d->handle))
-	    fabort ("unable to unload interim mapping of %W, %E",
-		    d->ntname);
+	    fabort ("unable to unload interim mapping of %W (using %W), %E",
+		    d->ntname, buffered_shortname (d->forkedntname ()));
 	}
       /* cygwin1.dll - as linked dependency - may reuse the shortname
 	 buffer, even in case of failure: don't reuse shortname later */
-      HMODULE h = LoadLibraryW (buffered_shortname (d->ntname));
+      HMODULE h = LoadLibraryW (buffered_shortname (d->forkedntname ()));
       if (!h)
-	fabort ("unable to map %W, %E", d->ntname);
+	fabort ("unable to map %W (using %W), %E",
+		d->ntname, buffered_shortname (d->forkedntname ()));
       if (h != d->handle)
-	fabort ("unable to map %W to same address as parent: %p != %p",
-		d->ntname, d->handle, h);
+	fabort ("unable to map %W (using %W) to same address as parent: %p != %p",
+		d->ntname, buffered_shortname (d->forkedntname ()), d->handle, h);
       /* Fix OS reference count. */
       for (int cnt = 1; cnt < d->count; ++cnt)
-	LoadLibraryW (buffered_shortname (d->ntname));
+	LoadLibraryW (buffered_shortname (d->forkedntname ()));
     }
 }
 
diff --git a/winsup/cygwin/dll_init.h b/winsup/cygwin/dll_init.h
index 62739f7..45f71cb 100644
--- a/winsup/cygwin/dll_init.h
+++ b/winsup/cygwin/dll_init.h
@@ -76,6 +76,10 @@ struct dll
 	p.run_dtors ();
       }
   }
+  PWCHAR forkedntname ()
+  {
+    return forkable_ntname && *forkable_ntname ? forkable_ntname : ntname;
+  }
 };
 
 #define MAX_DLL_BEFORE_INIT     100
@@ -98,6 +102,7 @@ class dll_list
   PWCHAR forkables_mutex_name;
   HANDLE forkables_mutex;
   void track_self ();
+  dll *find_by_forkedntname (PCWCHAR ntname);
   size_t forkable_ntnamesize (dll_type, PCWCHAR fullntname, PCWCHAR modname);
   void prepare_forkables_nomination ();
   void update_forkables_needs ();
diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index f929a85..530e423 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -315,12 +315,14 @@ frok::parent (volatile char * volatile stack_here)
 
   *with_forkables = dlls.setup_forkables (*with_forkables);
 
+  ch.silentfail (!*with_forkables); /* fail silently without forkables */
+
   while (1)
     {
       PCWCHAR forking_progname = NULL;
       if (dlls.main_executable)
         forking_progname = dll_list::buffered_shortname
-			   (dlls.main_executable->ntname);
+			   (dlls.main_executable->forkedntname ());
       if (!forking_progname || !*forking_progname)
 	forking_progname = myself->progname;
 
@@ -444,7 +446,7 @@ frok::parent (volatile char * volatile stack_here)
       impure_beg = _impure_ptr;
       impure_end = _impure_ptr + 1;
     }
-  rc = child_copy (hchild, true,
+  rc = child_copy (hchild, true, !*with_forkables,
 		   "stack", stack_here, ch.stackbase,
 		   impure, impure_beg, impure_end,
 		   NULL);
@@ -462,7 +464,7 @@ frok::parent (volatile char * volatile stack_here)
   for (dll *d = dlls.istart (DLL_LINK); d; d = dlls.inext ())
     {
       debug_printf ("copying data/bss of a linked dll");
-      if (!child_copy (hchild, true,
+      if (!child_copy (hchild, true, !*with_forkables,
 		       "linked dll data", d->p.data_start, d->p.data_end,
 		       "linked dll bss", d->p.bss_start, d->p.bss_end,
 		       NULL))
@@ -492,7 +494,7 @@ frok::parent (volatile char * volatile stack_here)
       for (dll *d = dlls.istart (DLL_LOAD); d; d = dlls.inext ())
 	{
 	  debug_printf ("copying data/bss for a loaded dll");
-	  if (!child_copy (hchild, true,
+	  if (!child_copy (hchild, true, !*with_forkables,
 			   "loaded dll data", d->p.data_start, d->p.data_end,
 			   "loaded dll bss", d->p.bss_start, d->p.bss_end,
 			   NULL))
@@ -539,7 +541,13 @@ cleanup:
 extern "C" int
 fork ()
 {
-  bool with_forkables = true;
+  bool with_forkables = false; /* do not force hardlinks on first try */
+  int res = dofork (&with_forkables);
+  if (res >= 0)
+    return res;
+  if (with_forkables)
+    return res; /* no need for second try when already enabled */
+  with_forkables = true; /* enable hardlinks for second try */
   return dofork (&with_forkables);
 }
 
@@ -613,6 +621,9 @@ dofork (bool *with_forkables)
     {
       if (!grouped.errmsg)
 	syscall_printf ("fork failed - child pid %d, errno %d", grouped.child_pid, grouped.this_errno);
+      else if (grouped.ch.silentfail ())
+	debug_printf ("child %d - %s, errno %d", grouped.child_pid,
+		       grouped.errmsg, grouped.this_errno);
       else
 	system_printf ("child %d - %s, errno %d", grouped.child_pid,
 		       grouped.errmsg, grouped.this_errno);
@@ -640,10 +651,10 @@ vfork ()
 /* Copy memory from one process to another. */
 
 bool
-child_copy (HANDLE hp, bool write, ...)
+child_copy (HANDLE hp, bool write, bool silentfail, ...)
 {
   va_list args;
-  va_start (args, write);
+  va_start (args, silentfail);
   static const char *huh[] = {"read", "write"};
 
   char *what;
@@ -669,10 +680,14 @@ child_copy (HANDLE hp, bool write, ...)
 	    {
 	      if (!res)
 		__seterrno ();
-	      /* If this happens then there is a bug in our fork
-		 implementation somewhere. */
-	      system_printf ("%s %s copy failed, %p..%p, done %lu, windows pid %u, %E",
-			    what, huh[write], low, high, done, myself->dwProcessId);
+	      if (silentfail)
+		debug_printf ("%s %s copy failed, %p..%p, done %lu, windows pid %u, %E",
+			     what, huh[write], low, high, done, myself->dwProcessId);
+	      else
+		/* If this happens then there is a bug in our fork
+		   implementation somewhere. */
+		system_printf ("%s %s copy failed, %p..%p, done %lu, windows pid %u, %E",
+			      what, huh[write], low, high, done, myself->dwProcessId);
 	      goto err;
 	    }
 	}
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index a830bff..30dfaaa 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -1101,7 +1101,10 @@ child_info_fork::abort (const char *fmt, ...)
     {
       va_list ap;
       va_start (ap, fmt);
-      strace_vprintf (SYSTEM, fmt, ap);
+      if (silentfail ())
+	strace_vprintf (DEBUG, fmt, ap);
+      else
+	strace_vprintf (SYSTEM, fmt, ap);
       TerminateProcess (GetCurrentProcess (), EXITCODE_FORK_FAILED);
     }
   if (retry > 0)
diff --git a/winsup/cygwin/winsup.h b/winsup/cygwin/winsup.h
index 3a5f5d5..95ab41e 100644
--- a/winsup/cygwin/winsup.h
+++ b/winsup/cygwin/winsup.h
@@ -224,7 +224,7 @@ int __small_swprintf (PWCHAR dst, const WCHAR *fmt, ...);
 int __small_vswprintf (PWCHAR dst, const WCHAR *fmt, va_list ap);
 void multiple_cygwin_problem (const char *, uintptr_t, uintptr_t);
 
-bool child_copy (HANDLE, bool, ...);
+bool child_copy (HANDLE, bool, bool, ...);
 
 class path_conv;



More information about the Cygwin-cvs mailing list