Implement fhandler reference counting.

* cygheap.h
(cygheap_fdmanip::release): Make virtual.
(cygheap_fdnew::~cygheap_fdnew): New destructor increments reference count when
fd has been allocated.
(cygheap_fdget::fh): New (old?) field.
(cygheap_fdget::cygheap_fdget): Increment reference count when we've found an
active fd.  Set fh appropriately.
(cygheap_fdget::~cygheap_fdget): Decrement reference count when appropriate.
Delete fh if reference count goes to zero.
(cygheap_fdget::release): New function.  Do more bookkeping on release.
* dtable.cc (dtable::release): Change from void to boolean return.  Only delete
the fhandler when its reference count is <= 0 (this should be a fairly unusual
case).  Return true if fhandler has been deleted.
(cygwin_attach_handle_to_fd): Increment reference count when fh is assigned.
(dtable::init_std_file_from_handle): Ditto.
* dtable.h (dtable::release): Change return to boolean.
* fhandler.cc (fhandler_base::fhandler_base): Set new isclosed flag to false.
Set _refcnt to zero.
(fhandler_base::close): Simplify paranoid debugging output.  Set new isclosed()
flag.
(fhandler_base_overlapped::wait_overlapped): Use isclosed() flag to avoid
querying the exception handle.
* fhandler.h (fhandler_base::_refcnt): New field.
(fhandler_base::refcnt): New function.
(fhandler_base::isclosed): Implement.
(fhandler_base::fhandler_base): Set isclosed to false.
* syscalls.cc: Remove space after function before parentheses for several
strace printfs.
(dup): Add standard strace "leaver" code.
(dup2): Ditto.
(dup3): Ditto.
(remove): Ditto.
(getpid): Ditto.
(getppid): Ditto.
(lseek64): Fix strace debugging to correctly use %R.
* fhandler_termios.cc (fhandler_termios::tcsetpgrp): Avoid sending signals to
other processes if we're debugging since it can cause a deadlock with the
calling debugger.
* exceptions.cc (_cygtls::call_signal_handler): Add debugging-only strace
output.

9 files changed, 141 insertions, 46 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 7167510b2..b5f5d82cf 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,51 @@
+2011-12-16  Christopher Faylor  <me.cygwin2011@cgf.cx>
+
+	Implement fhandler reference counting.
+	* cygheap.h
+	(cygheap_fdmanip::release): Make virtual.
+	(cygheap_fdnew::~cygheap_fdnew): New destructor increments reference
+	count when fd has been allocated.
+	(cygheap_fdget::fh): New (old?) field.
+	(cygheap_fdget::cygheap_fdget): Increment reference count when we've
+	found an active fd.  Set fh appropriately.
+	(cygheap_fdget::~cygheap_fdget): Decrement reference count when
+	appropriate.  Delete fh if reference count goes to zero.
+	(cygheap_fdget::release): New function.  Do more bookkeping on release.
+	* dtable.cc (dtable::release): Change from void to boolean return.
+	Only delete the fhandler when its reference count is <= 0 (this should
+	be a fairly unusual case).  Return true if fhandler has been deleted.
+	(cygwin_attach_handle_to_fd): Increment reference count when fh is
+	assigned.
+	(dtable::init_std_file_from_handle): Ditto.
+	* dtable.h (dtable::release): Change return to boolean.
+	* fhandler.cc (fhandler_base::fhandler_base): Set new isclosed flag to
+	false.  Set _refcnt to zero.
+	(fhandler_base::close): Simplify paranoid debugging output.  Set new
+	isclosed() flag.
+	(fhandler_base_overlapped::wait_overlapped): Use isclosed() flag to
+	avoid querying the exception handle.
+	* fhandler.h (fhandler_base::_refcnt): New field.
+	(fhandler_base::refcnt): New function.
+	(fhandler_base::isclosed): Implement.
+	(fhandler_base::fhandler_base): Set isclosed to false.
+
+	* syscalls.cc: Remove space after function before parentheses for
+	several strace printfs.
+	(dup): Add standard strace "leaver" code.
+	(dup2): Ditto.
+	(dup3): Ditto.
+	(remove): Ditto.
+	(getpid): Ditto.
+	(getppid): Ditto.
+	(lseek64): Fix strace debugging to correctly use %R.
+
+	* fhandler_termios.cc (fhandler_termios::tcsetpgrp): Avoid sending
+	signals to other processes if we're debugging since it can cause a
+	deadlock with the calling debugger.
+
+	* exceptions.cc (_cygtls::call_signal_handler): Add debugging-only
+	strace output.
+
 2011-12-16  Corinna Vinschen  <vinschen@redhat.com>
 
 	* dcrt0.cc (child_info_fork::alloc_stack): Correctly check if the
diff --git a/winsup/cygwin/cygheap.h b/winsup/cygwin/cygheap.h
index 204ebe2f8..5d7b93dcc 100644
--- a/winsup/cygwin/cygheap.h
+++ b/winsup/cygwin/cygheap.h
@@ -328,10 +328,7 @@ class cygheap_fdmanip
     if (locked)
       cygheap->fdtab.unlock ();
   }
-  void release ()
-  {
-    cygheap->fdtab.release (fd);
-  }
+  virtual void release () { cygheap->fdtab.release (fd); }
   operator int &() {return fd;}
   operator fhandler_base* &() {return cygheap->fdtab[fd];}
   operator fhandler_socket* () const {return reinterpret_cast<fhandler_socket *> (cygheap->fdtab[fd]);}
@@ -368,12 +365,18 @@ class cygheap_fdnew : public cygheap_fdmanip
 	locked = false;
       }
   }
+  ~cygheap_fdnew ()
+  {
+    if (cygheap->fdtab[fd])
+      cygheap->fdtab[fd]->refcnt (1);
+  }
   void operator = (fhandler_base *fh) {cygheap->fdtab[fd] = fh;}
 };
 
 class cygheap_fdget : public cygheap_fdmanip
 {
- public:
+  fhandler_base *fh;
+public:
   cygheap_fdget (int fd, bool lockit = false, bool do_set_errno = true)
   {
     if (lockit)
@@ -382,6 +385,8 @@ class cygheap_fdget : public cygheap_fdmanip
       {
 	this->fd = fd;
 	locked = lockit;
+	fh = cygheap->fdtab[fd];
+	fh->refcnt (1);
       }
     else
       {
@@ -391,8 +396,27 @@ class cygheap_fdget : public cygheap_fdmanip
 	if (lockit)
 	  cygheap->fdtab.unlock ();
 	locked = false;
+	fh = NULL;
       }
   }
+  ~cygheap_fdget ()
+  {
+    if (!fh)
+      /* nothing to do */;
+    else if (fh->refcnt (-1) > 0)
+      debug_only_printf ("fh %p, %s, refcnt %ld", fh, fh->get_name (), fh->refcnt ());
+    else
+      {
+	debug_only_printf ("deleting fh %p, %s, refcnt %ld", fh, fh->get_name (), fh->refcnt ());
+	delete fh;
+      }
+  }
+  void release ()
+  {
+    fh = cygheap->fdtab[fd];
+    if (cygheap->fdtab.release (fd))
+      fh = NULL;
+  }
 };
 
 class cygheap_fdenum : public cygheap_fdmanip
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 4da3a908b..c8619ca84 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -241,16 +241,26 @@ dtable::find_unused_handle (int start)
   return -1;
 }
 
-void
+bool
 dtable::release (int fd)
 {
-  if (!not_open (fd))
+  bool deleted;
+  if (not_open (fd))
+    deleted = false;
+  else
     {
       if (fds[fd]->need_fixup_before ())
 	dec_need_fixup_before ();
-      delete fds[fd];
+      if (fds[fd]->refcnt (-1) > 0)
+	deleted = false;
+      else
+	{
+	  deleted = true;
+	  delete fds[fd];
+	}
       fds[fd] = NULL;
     }
+  return deleted;
 }
 
 extern "C" int
@@ -261,6 +271,7 @@ cygwin_attach_handle_to_fd (char *name, int fd, HANDLE handle, mode_t bin,
     fd = cygheap->fdtab.find_unused_handle ();
   fhandler_base *fh = build_fh_name (name);
   cygheap->fdtab[fd] = fh;
+  cygheap->fdtab[fd]->refcnt (1);
   fh->init (handle, myaccess, bin ?: fh->pc_binmode ());
   return fd;
 }
@@ -395,6 +406,7 @@ dtable::init_std_file_from_handle (int fd, HANDLE handle)
       fh->open_setup (openflags);
       fh->usecount = 0;
       cygheap->fdtab[fd] = fh;
+      cygheap->fdtab[fd]->refcnt (1);
       set_std_handle (fd);
       paranoid_printf ("fd %d, handle %p", fd, handle);
     }
diff --git a/winsup/cygwin/dtable.h b/winsup/cygwin/dtable.h
index 1c487d131..cab9ac577 100644
--- a/winsup/cygwin/dtable.h
+++ b/winsup/cygwin/dtable.h
@@ -63,7 +63,7 @@ public:
   }
   int find_unused_handle (int start);
   int find_unused_handle () { return find_unused_handle (first_fd_for_open);}
-  void release (int fd);
+  bool release (int fd) __attribute__ ((regparm (2)));
   void init_std_file_from_handle (int fd, HANDLE handle);
   int dup3 (int oldfd, int newfd, int flags);
   void fixup_after_exec ();
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 9250b61f2..3fcd91c5e 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1344,6 +1344,7 @@ _cygtls::call_signal_handler ()
       if (!sig)
 	break;
 
+      debug_only_printf ("dealing with signal %d", sig);
       this_sa_flags = sa_flags;
       int thissig = sig;
       void (*thisfunc) (int) = func;
diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
index 5fd416ce2..04499e6e7 100644
--- a/winsup/cygwin/fhandler.cc
+++ b/winsup/cygwin/fhandler.cc
@@ -1148,11 +1148,10 @@ fhandler_base::close ()
     res = 0;
   else
     {
-      paranoid_printf ("CloseHandle (%d <%s>) failed", get_handle (),
-		       get_name ());
-
+      paranoid_printf ("CloseHandle failed, %E");
       __seterrno ();
     }
+  isclosed (true);
   return res;
 }
 
@@ -1485,6 +1484,7 @@ fhandler_base::fhandler_base () :
   access (0),
   io_handle (NULL),
   ino (0),
+  _refcnt (0),
   openflags (0),
   rabuf (NULL),
   ralen (0),
@@ -1495,6 +1495,7 @@ fhandler_base::fhandler_base () :
   archetype (NULL),
   usecount (0)
 {
+  isclosed (false);
 }
 
 /* Normal I/O destructor */
@@ -1928,8 +1929,8 @@ fhandler_base_overlapped::wait_overlapped (bool inres, bool writing, DWORD *byte
       DWORD wfres = cygwait (get_overlapped ()->hEvent);
       HANDLE h = writing ? get_output_handle () : get_handle ();
       BOOL wores;
-      if (wfres == WAIT_OBJECT_0 + 1 && !get_overlapped ())
-	wores = 0;
+      if (isclosed ())
+	wores = 0;	/* closed in another thread or via signal handler */
       else
 	{
 	  /* Cancelling here to prevent races.  It's possible that the I/O has
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index b1324c563..0a73af82f 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -142,12 +142,13 @@ class fhandler_base
 					read or write access */
     unsigned close_on_exec      : 1; /* close-on-exec */
     unsigned need_fork_fixup    : 1; /* Set if need to fixup after fork. */
+    unsigned isclosed		: 1; /* Set when fhandler is closed. */
 
    public:
     status_flags () :
       rbinary (0), rbinset (0), wbinary (0), wbinset (0), nohandle (0),
       did_lseek (0), query_open (no_query), close_on_exec (0),
-      need_fork_fixup (0)
+      need_fork_fixup (0), isclosed (0)
       {}
   } status, open_status;
 
@@ -158,6 +159,7 @@ class fhandler_base
   HANDLE io_handle;
 
   __ino64_t ino;	/* file ID or hashed filename, depends on FS. */
+  long _refcnt;
 
  protected:
   /* File open flags from open () and fcntl () calls */
@@ -176,6 +178,7 @@ class fhandler_base
   HANDLE read_state;
 
  public:
+  long refcnt(long i = 0) {return _refcnt += i;}
   class fhandler_base *archetype;
   int usecount;
 
@@ -239,6 +242,7 @@ class fhandler_base
   IMPLEMENT_STATUS_FLAG (query_state, query_open)
   IMPLEMENT_STATUS_FLAG (bool, close_on_exec)
   IMPLEMENT_STATUS_FLAG (bool, need_fork_fixup)
+  IMPLEMENT_STATUS_FLAG (bool, isclosed)
 
   int get_default_fmode (int flags);
 
diff --git a/winsup/cygwin/fhandler_termios.cc b/winsup/cygwin/fhandler_termios.cc
index 7dc5bf58c..fca180d99 100644
--- a/winsup/cygwin/fhandler_termios.cc
+++ b/winsup/cygwin/fhandler_termios.cc
@@ -78,7 +78,7 @@ fhandler_termios::tcsetpgrp (const pid_t pgid)
 	{
 	case bg_ok:
 	  tc ()->setpgid (pgid);
-	  if (tc ()->is_console)
+	  if (tc ()->is_console && (strace.active () || !being_debugged ()))
 	    tc ()->kill_pgrp (__SIGSETPGRP);
 	  res = 0;
 	  break;
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 6ac4b1c97..f77c451bf 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -120,51 +120,51 @@ close_all_files (bool norelease)
 extern "C" int
 dup (int fd)
 {
-  return cygheap->fdtab.dup3 (fd, cygheap_fdnew (), 0);
+  int res = cygheap->fdtab.dup3 (fd, cygheap_fdnew (), 0);
+  syscall_printf ("%R = dup(%d)", res, fd);
+  return res;
 }
 
 extern "C" int
 dup2 (int oldfd, int newfd)
 {
+  int res;
   if (newfd >= OPEN_MAX_MAX)
     {
-      syscall_printf ("-1 = dup2 (%d, %d) (%d too large)", oldfd, newfd, newfd);
       set_errno (EBADF);
-      return -1;
+      res = -1;
     }
-  if (newfd == oldfd)
+  else if (newfd == oldfd)
     {
       cygheap_fdget cfd (oldfd);
-      if (cfd < 0)
-	{
-	  syscall_printf ("%R = dup2(%d,%d) (oldfd not open)", -1, oldfd, newfd);
-	  return -1;
-	}
-      syscall_printf ("%R = dup2(%d, %d) (newfd==oldfd)", oldfd, oldfd, newfd);
-      return oldfd;
+      res = (cfd >= 0) ? oldfd : -1;
     }
-  return cygheap->fdtab.dup3 (oldfd, newfd, 0);
+  else
+    res = cygheap->fdtab.dup3 (oldfd, newfd, 0); 
+
+  syscall_printf ("%R = dup2(%d, %d)", res, oldfd, newfd);
+  return res;
 }
 
 extern "C" int
 dup3 (int oldfd, int newfd, int flags)
 {
+  int res;
   if (newfd >= OPEN_MAX_MAX)
     {
-      syscall_printf ("-1 = dup3 (%d, %d, %p) (%d too large)",
-		      oldfd, newfd, flags, newfd);
       set_errno (EBADF);
-      return -1;
+      res = -1;
     }
-  if (newfd == oldfd)
+  else if (newfd == oldfd)
     {
       cygheap_fdget cfd (oldfd, false, false);
       set_errno (cfd < 0 ? EBADF : EINVAL);
-      syscall_printf ("-1 = dup3 (%d, %d, %p) (newfd==oldfd)",
-		      oldfd, newfd, flags);
-      return -1;
+      res = -1;
     }
-  return cygheap->fdtab.dup3 (oldfd, newfd, flags);
+  else
+    res = cygheap->fdtab.dup3 (oldfd, newfd, flags);
+  syscall_printf ("%R = dup2(%d, %d, %p)", res, oldfd, newfd, flags);
+  return res;
 }
 
 static inline void
@@ -965,12 +965,15 @@ remove (const char *ourname)
       return -1;
     }
 
-  return win32_name.isdir () ? rmdir (ourname) : unlink (ourname);
+  int res = win32_name.isdir () ? rmdir (ourname) : unlink (ourname);
+  syscall_printf ("%R = remove(%s)", res, ourname);
+  return res;
 }
 
 extern "C" pid_t
 getpid ()
 {
+  syscall_printf ("%d = getpid()", myself->pid);
   return myself->pid;
 }
 
@@ -984,6 +987,7 @@ _getpid_r (struct _reent *)
 extern "C" pid_t
 getppid ()
 {
+  syscall_printf ("%d = getppid()", myself->ppid);
   return myself->ppid;
 }
 
@@ -1041,6 +1045,7 @@ getsid (pid_t pid)
 	  res = -1;
 	}
     }
+  syscall_printf ("%R = getsid(%d)", pid);
   return res;
 }
 
@@ -1066,7 +1071,7 @@ read (int fd, void *ptr, size_t len)
     }
 
   /* Could block, so let user know we at least got here.  */
-  syscall_printf ("read (%d, %p, %d) %sblocking",
+  syscall_printf ("read(%d, %p, %d) %sblocking",
 		  fd, ptr, len, cfd->is_nonblocking () ? "non" : "");
 
   cfd->read (ptr, res = len);
@@ -1108,7 +1113,7 @@ readv (int fd, const struct iovec *const iov, const int iovcnt)
     }
 
   /* Could block, so let user know we at least got here.  */
-  syscall_printf ("readv (%d, %p, %d) %sblocking",
+  syscall_printf ("readv(%d, %p, %d) %sblocking",
 		  fd, iov, iovcnt, cfd->is_nonblocking () ? "non" : "");
 
   res = cfd->readv (iov, iovcnt, tot);
@@ -1158,9 +1163,9 @@ write (int fd, const void *ptr, size_t len)
 
   /* Could block, so let user know we at least got here.  */
   if (fd == 1 || fd == 2)
-    paranoid_printf ("write (%d, %p, %d)", fd, ptr, len);
+    paranoid_printf ("write(%d, %p, %d)", fd, ptr, len);
   else
-    syscall_printf  ("write (%d, %p, %d)", fd, ptr, len);
+    syscall_printf  ("write(%d, %p, %d)", fd, ptr, len);
 
   res = cfd->write (ptr, len);
 
@@ -1206,9 +1211,9 @@ writev (const int fd, const struct iovec *const iov, const int iovcnt)
 
   /* Could block, so let user know we at least got here.  */
   if (fd == 1 || fd == 2)
-    paranoid_printf ("writev (%d, %p, %d)", fd, iov, iovcnt);
+    paranoid_printf ("writev(%d, %p, %d)", fd, iov, iovcnt);
   else
-    syscall_printf  ("writev (%d, %p, %d)", fd, iov, iovcnt);
+    syscall_printf  ("writev(%d, %p, %d)", fd, iov, iovcnt);
 
   res = cfd->writev (iov, iovcnt, tot);
 
@@ -1248,7 +1253,7 @@ open (const char *unix_path, int flags, ...)
   va_list ap;
   mode_t mode = 0;
 
-  syscall_printf ("open (%s, %p)", unix_path, flags);
+  syscall_printf ("open(%s, %p)", unix_path, flags);
   pthread_testcancel ();
   myfault efault;
   if (efault.faulted (EFAULT))
@@ -1341,7 +1346,7 @@ lseek64 (int fd, _off64_t pos, int dir)
       else
 	res = -1;
     }
-  syscall_printf ("%D = lseek (%d, %D, %d)", res, fd, pos, dir);
+  syscall_printf ("%R = lseek(%d, %D, %d)", res, fd, pos, dir);
 
   return res;
 }
@@ -1361,7 +1366,7 @@ close (int fd)
 {
   int res;
 
-  syscall_printf ("close (%d)", fd);
+  syscall_printf ("close(%d)", fd);
 
   pthread_testcancel ();