[PATCH RFA] Retry open of /proc/PID/ctl in procfs.c

[PATCH RFA] Retry open of /proc/PID/ctl in procfs.c

[Kevin Buettner]

I request approval for committing the patch below.  The comments in the
patch explain why this patch is needed.  (It definitely cures some
intermittent problems that I'm seeing on AIX5.)

Tested on i586-sco-sysv5uw7.1.1 and i386-pc-solaris2.8.  No regressions.

	* procfs.c (procfs_init_inferior): Add code for retrying the
	open of the /proc control file for the newly created process.

Index: procfs.c
===================================================================
RCS file: /cvs/src/src/gdb/procfs.c,v
retrieving revision 1.26
diff -u -p -r1.26 procfs.c
--- procfs.c	2001/03/27 02:01:11	1.26
+++ procfs.c	2001/04/13 07:57:26
@@ -4773,6 +4773,7 @@ procfs_init_inferior (int pid)
   procinfo *pi;
   gdb_sigset_t signals;
   int fail;
+  int retry_count;
 
   /* This routine called on the parent side (GDB side)
      after GDB forks the inferior.  */
@@ -4782,8 +4783,25 @@ procfs_init_inferior (int pid)
   if ((pi = create_procinfo (pid, 0)) == NULL)
     perror ("procfs: out of memory in 'init_inferior'");
 
-  if (!open_procinfo_files (pi, FD_CTL))
-    proc_error (pi, "init_inferior, open_proc_files", __LINE__);
+  /* Open the /proc control (ctl) file, but be prepared to retry
+     this open in the event that it hasn't been created yet.  */
+  retry_count = 3;
+  while (!open_procinfo_files (pi, FD_CTL))
+    {
+      if (retry_count == 0)
+	{
+	  /* Retry count exceeded; print error and return to top level.  */
+	  proc_error (pi, "init_inferior, open_proc_files", __LINE__);
+	}
+      else
+	{
+	  /* In some /proc implementations, GDB tries to open /proc/PID/ctl
+	     before the kernel has created it.  Sleep for a bit to give
+	     the kernel a chance to catch up.  */
+	  sleep (1);
+	  retry_count--;
+	}
+    }
 
   /*
     xmalloc			// done

Re: [PATCH RFA] Retry open of /proc/PID/ctl in procfs.c

[Kevin Buettner]

On Apr 13,  3:23pm, Michael Snyder wrote:

> Kevin Buettner wrote:
> > 
> > I request approval for committing the patch below.  The comments in the
> > patch explain why this patch is needed.  (It definitely cures some
> > intermittent problems that I'm seeing on AIX5.)
> 
> Is this just an issue of avoiding a race condition between gdb opening
> the "file" and the kernel creating it?

Yes.  I believe the problem that I'm seeing on AIX5 is due to a race
condition.  The race is between GDB and the kernel to see who can
open/create /proc/PID/ctl first.  The kernel usually wins this race,
but roughly one time out of fifty, GDB wins it instead.

> I can certainly imagine this being an issue on any platform, so I'm
> glad you didn't ifdef it, but I wonder if this is the only place
> where it might show up?

I think similar problems show up elsewhere too.  E.g, see

    http://sources.redhat.com/ml/gdb/2001-04/msg00078.html

In this bug report, Damien Kick reports that...

    'truss'ing the running 'gdb' shows that the failure in '/proc'
    comes from an attempt to 'open' '/proc/pid/status' that fails with
    EAGAIN.

This is on sparc-sun-solaris2.7.

The problem that Damien reported is not precisely the same as the
problem that I'm seeing on AIX5, but retrying the open() seems like
the right thing to do for Damien's problem too.

> Would it be worth it to add a "nohang" argument to open_procinfo_files?

It did occur to me that it might be better to move the retry logic
into open_procinfo_files().  I suppose the "nohang" argument you refer
to would only cause the open to be attempted only once?

> If not, then I say this change is fine.

I'll rework my patch so that the retry logic is located in
open_procinfo_files() instead and resubmit it.

Thanks for your feedback.

Kevin