[PATCH draft v2 5/6] Cygwin: remove the fhandler_base_overlapped class

[Ken Brown <kbrown at cornell dot edu>]

There are no longer any classes derived from it.

Also remove the 'was_nonblocking' flag, which was needed only for
fhandler_base_overlapped.
---
 winsup/cygwin/fhandler.cc | 383 --------------------------------------
 winsup/cygwin/fhandler.h  |  75 +-------
 winsup/cygwin/syscalls.cc |   1 -
 3 files changed, 1 insertion(+), 458 deletions(-)

diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
index b0c9c50c3..bb618fa88 100644
--- a/winsup/cygwin/fhandler.cc
+++ b/winsup/cygwin/fhandler.cc
@@ -30,16 +30,10 @@ details. */
 #include <asm/socket.h>
 #include "cygwait.h"
 
-#define MAX_OVERLAPPED_WRITE_LEN (64 * 1024 * 1024)
-#define MIN_OVERLAPPED_WRITE_LEN (1 * 1024 * 1024)
-
 static const int CHUNK_SIZE = 1024; /* Used for crlf conversions */
 
 struct __cygwin_perfile *perfile_table;
 
-HANDLE NO_COPY fhandler_base_overlapped::asio_done;
-LONG NO_COPY fhandler_base_overlapped::asio_close_counter;
-
 void
 fhandler_base::reset (const fhandler_base *from)
 {
@@ -196,8 +190,6 @@ fhandler_base::set_flags (int flags, int supplied_bin)
     bin = wbinary () || rbinary () ? O_BINARY : O_TEXT;
 
   openflags = flags | bin;
-  if (openflags & O_NONBLOCK)
-    was_nonblocking (true);
 
   bin &= O_BINARY;
   rbinary (bin ? true : false);
@@ -1223,88 +1215,6 @@ fhandler_base::close ()
   return res;
 }
 
-DWORD WINAPI
-flush_async_io (void *arg)
-{
-  fhandler_base_overlapped *fh = (fhandler_base_overlapped *) arg;
-  debug_only_printf ("waiting for write I/O for %s", fh->get_name ());
-  DWORD nbytes;
-  bool res = GetOverlappedResult (fh->get_output_handle (),
-				  fh->get_overlapped (), &nbytes, true);
-  debug_printf ("finished waiting for I/O from %s, res %d", fh->get_name (),
-		res);
-  fh->close ();
-  delete fh;
-
-  InterlockedDecrement (&fhandler_base_overlapped::asio_close_counter);
-  SetEvent (fhandler_base_overlapped::asio_done);
-
-  _my_tls._ctinfo->auto_release ();
-  return 0;
-}
-
-void
-fhandler_base_overlapped::flush_all_async_io ()
-{
-  while (asio_close_counter > 0)
-    if (WaitForSingleObject (asio_done, INFINITE) != WAIT_OBJECT_0)
-      {
-	system_printf ("WaitForSingleObject failed, possible data loss in pipe, %E");
-	break;
-      }
-  asio_close_counter = 0;
-  if (asio_done)
-    CloseHandle (asio_done);
-}
-
-/* Start a thread to handle closing of overlapped asynchronous I/O since
-   Windows amazingly does not seem to always flush I/O on close.  */
-void
-fhandler_base_overlapped::check_later ()
-{
-  set_close_on_exec (true);
-  char buf[MAX_PATH];
-  if (!asio_done
-      && !(asio_done = CreateEvent (&sec_none_nih, false, false,
-				    shared_name (buf, "asio",
-						 GetCurrentProcessId ()))))
-    api_fatal ("CreateEvent failed, %E");
-
-  InterlockedIncrement (&asio_close_counter);
-  if (!new cygthread(flush_async_io, this, "flasio"))
-    api_fatal ("couldn't create a thread to track async I/O, %E");
-  debug_printf ("started thread to handle asynchronous closing for %s", get_name ());
-}
-
-int
-fhandler_base_overlapped::close ()
-{
-  int res;
-  int writer = (get_access () & GENERIC_WRITE);
-  /* Need to treat non-blocking I/O specially because Windows appears to
-     be brain-dead.  We're checking here if the descriptor was ever set
-     to nonblocking, rather than checking if it's nonblocking at close time.
-     The reason is that applications may switch back to blocking (for the
-     sake of some other application accessing this descriptor) without
-     performaing any further I/O.  These applications would suffer data
-     loss, which this workaround is trying to fix. */
-  if (writer && was_nonblocking () && has_ongoing_io ())
-    {
-      clone (HEAP_3_FHANDLER)->check_later ();
-      res = 0;
-    }
-  else
-    {
-     /* Cancelling seems to be necessary for cases where a reader is
-	 still executing when a signal handler performs a close.  */
-      if (!writer)
-	CancelIo (get_handle ());
-      destroy_overlapped ();
-      res = fhandler_base::close ();
-    }
-  return res;
-}
-
 int
 fhandler_base::ioctl (unsigned int cmd, void *buf)
 {
@@ -1422,14 +1332,6 @@ fhandler_base::dup (fhandler_base *child, int)
   return 0;
 }
 
-int
-fhandler_base_overlapped::dup (fhandler_base *child, int flags)
-{
-  int res = fhandler_base::dup (child, flags) ||
-	    ((fhandler_base_overlapped *) child)->setup_overlapped ();
-  return res;
-}
-
 int fhandler_base::fcntl (int cmd, intptr_t arg)
 {
   int res;
@@ -1649,13 +1551,6 @@ fhandler_base::fixup_after_fork (HANDLE parent)
     del_my_locks (after_fork);
 }
 
-void
-fhandler_base_overlapped::fixup_after_fork (HANDLE parent)
-{
-  setup_overlapped ();
-  fhandler_base::fixup_after_fork (parent);
-}
-
 void
 fhandler_base::fixup_after_exec ()
 {
@@ -1664,12 +1559,6 @@ fhandler_base::fixup_after_exec ()
     del_my_locks (after_exec);
   mandatory_locking (false);
 }
-void
-fhandler_base_overlapped::fixup_after_exec ()
-{
-  setup_overlapped ();
-  fhandler_base::fixup_after_exec ();
-}
 
 bool
 fhandler_base::is_nonblocking ()
@@ -1683,8 +1572,6 @@ fhandler_base::set_nonblocking (int yes)
   int current = openflags & O_NONBLOCK_MASK;
   int new_flags = yes ? (!current ? O_NONBLOCK : current) : 0;
   openflags = (openflags & ~O_NONBLOCK_MASK) | new_flags;
-  if (new_flags)
-    was_nonblocking (true);
 }
 
 int
@@ -1948,273 +1835,3 @@ fhandler_base::fpathconf (int v)
     }
   return -1;
 }
-
-/* Overlapped I/O */
-
-int __reg1
-fhandler_base_overlapped::setup_overlapped ()
-{
-  OVERLAPPED *ov = get_overlapped_buffer ();
-  memset (ov, 0, sizeof (*ov));
-  set_overlapped (ov);
-  ov->hEvent = CreateEvent (&sec_none_nih, true, true, NULL);
-  io_pending = false;
-  return ov->hEvent ? 0 : -1;
-}
-
-void __reg1
-fhandler_base_overlapped::destroy_overlapped ()
-{
-  OVERLAPPED *ov = get_overlapped ();
-  if (ov && ov->hEvent)
-    {
-      SetEvent (ov->hEvent);
-      CloseHandle (ov->hEvent);
-      ov->hEvent = NULL;
-    }
-  io_pending = false;
-  get_overlapped () = NULL;
-}
-
-bool __reg1
-fhandler_base_overlapped::has_ongoing_io ()
-{
-  if (!io_pending)
-    return false;
-
-  if (!IsEventSignalled (get_overlapped ()->hEvent))
-    return true;
-  io_pending = false;
-  DWORD nbytes;
-  GetOverlappedResult (get_output_handle (), get_overlapped (), &nbytes, false);
-  return false;
-}
-
-fhandler_base_overlapped::wait_return __reg3
-fhandler_base_overlapped::wait_overlapped (bool inres, bool writing, DWORD *bytes, bool nonblocking, DWORD len)
-{
-  if (!get_overlapped ())
-    return inres ? overlapped_success : overlapped_error;
-
-  wait_return res = overlapped_unknown;
-  DWORD err;
-  if (inres)
-    /* handle below */;
-  else if ((err = GetLastError ()) != ERROR_IO_PENDING)
-    res = overlapped_error;
-  else if (!nonblocking)
-    /* handle below */;
-  else if (!writing)
-    SetEvent (get_overlapped ()->hEvent);	/* Force immediate WFMO return */
-  else
-    {
-      *bytes = len;				/* Assume that this worked */
-      io_pending = true;			/*  but don't allow subsequent */
-      res = overlapped_success;			/*  writes until completed */
-    }
-  if (res == overlapped_unknown)
-    {
-      DWORD wfres = cygwait (get_overlapped ()->hEvent);
-      HANDLE h = writing ? get_output_handle () : get_handle ();
-      BOOL wores;
-      if (isclosed ())
-	{
-	  switch (wfres)
-	    {
-	    case WAIT_OBJECT_0:
-	      err = ERROR_INVALID_HANDLE;
-	      break;
-	    case WAIT_SIGNALED:
-	      err = ERROR_INVALID_AT_INTERRUPT_TIME;
-	      break;
-	    default:
-	      err = GetLastError ();
-	      break;
-	    }
-	  res = overlapped_error;
-	}
-      else
-	{
-	  /* Cancelling here to prevent races.  It's possible that the I/O has
-	     completed already, in which case this is a no-op.  Otherwise,
-	     WFMO returned because 1) This is a non-blocking call, 2) a signal
-	     arrived, or 3) the operation was cancelled.  These cases may be
-	     overridden by the return of GetOverlappedResult which could detect
-	     that I/O completion occurred.  */
-	  CancelIo (h);
-	  wores = GetOverlappedResult (h, get_overlapped (), bytes, false);
-	  err = GetLastError ();
-	  ResetEvent (get_overlapped ()->hEvent);	/* Probably not needed but CYA */
-	  debug_printf ("wfres %u, wores %d, bytes %u", wfres, wores, *bytes);
-	  if (wores)
-	    res = overlapped_success;	/* operation succeeded */
-	  else if (wfres == WAIT_OBJECT_0 + 1)
-	    {
-	      err = ERROR_INVALID_AT_INTERRUPT_TIME; /* forces an EINTR below */
-	      debug_printf ("signal");
-	      res = overlapped_error;
-	    }
-	  else if (wfres == WAIT_OBJECT_0 + 2)
-	    pthread::static_cancel_self ();		/* never returns */
-	  else if (nonblocking)
-	    res = overlapped_nonblocking_no_data;	/* more handling below */
-	  else
-	    {
-	      debug_printf ("GetOverlappedResult failed, h %p, bytes %u, %E", h, *bytes);
-	      res = overlapped_error;
-	    }
-	}
-    }
-
-  if (res == overlapped_success)
-    {
-      debug_printf ("normal %s, %u bytes ispipe() %d", writing ? "write" : "read", *bytes, ispipe ());
-      if (*bytes == 0 && !writing && ispipe ())
-	res = overlapped_nullread;
-    }
-  else if (res == overlapped_nonblocking_no_data)
-    {
-      *bytes = (DWORD) -1;
-      set_errno (EAGAIN);
-      debug_printf ("no data to read for nonblocking I/O");
-    }
-  else if (err == ERROR_HANDLE_EOF || err == ERROR_BROKEN_PIPE)
-    {
-      debug_printf ("EOF, %E");
-      *bytes = 0;
-      res = overlapped_success;
-      if (writing && err == ERROR_BROKEN_PIPE)
-	raise (SIGPIPE);
-    }
-  else
-    {
-      debug_printf ("res %u, Win32 Error %u", (unsigned) res, err);
-      *bytes = (DWORD) -1;
-      __seterrno_from_win_error (err);
-      if (writing && err == ERROR_NO_DATA)
-	raise (SIGPIPE);
-    }
-
-  return res;
-}
-
-void __reg3
-fhandler_base_overlapped::raw_read (void *ptr, size_t& len)
-{
-  DWORD nbytes;
-  bool keep_looping;
-  do
-    {
-      bool res = ReadFile (get_handle (), ptr, len, &nbytes,
-			   get_overlapped ());
-      switch (wait_overlapped (res, false, &nbytes, is_nonblocking ()))
-	{
-	case overlapped_nullread:
-	  keep_looping = true;
-	  break;
-	default:	/* Added to quiet gcc */
-	case overlapped_success:
-	case overlapped_error:
-	  keep_looping = false;
-	  break;
-	}
-    }
-  while (keep_looping);
-  len = (nbytes == (DWORD) -1) ? (size_t) -1 : (size_t) nbytes;
-}
-
-ssize_t __reg3
-fhandler_base_overlapped::raw_write (const void *ptr, size_t len)
-{
-  size_t nbytes;
-  if (has_ongoing_io ())
-    {
-      set_errno (EAGAIN);
-      nbytes = (size_t) -1;
-    }
-  else
-    {
-      size_t chunk;
-      if (!max_atomic_write || len < max_atomic_write)
-	chunk = MIN (len, INT_MAX);
-      else if (is_nonblocking ())
-	chunk = len = max_atomic_write;
-      else
-	chunk = max_atomic_write;
-
-      /* MSDN "WriteFile" contains the following note: "Accessing the output
-         buffer while a write operation is using the buffer may lead to
-	 corruption of the data written from that buffer.  [...]  This can
-	 be particularly problematic when using an asynchronous file handle.
-	 (https://msdn.microsoft.com/en-us/library/windows/desktop/aa365747)
-
-	 MSDN "Synchronous and Asynchronous I/O" contains the following note:
-	 "Do not deallocate or modify [...] the data buffer until all
-	 asynchronous I/O operations to the file object have been completed."
-	 (https://msdn.microsoft.com/en-us/library/windows/desktop/aa365683)
-
-	 This problem is a non-issue for blocking I/O, but it can lead to
-	 problems when using nonblocking I/O.  Consider:
-	 - The application uses a static buffer in repeated calls to
-	   non-blocking write.
-	 - The previous write returned with success, but the overlapped I/O
-	   operation is ongoing.
-	 - The application copies the next set of data to the static buffer,
-	   thus overwriting data still accessed by the previous write call.
-	 --> potential data corruption.
-
-	 What we do here is to allocate a per-fhandler buffer big enough
-	 to perform the maximum atomic operation from, copy the user space
-	 data over to this buffer and then call NtWriteFile on this buffer.
-	 This decouples the write operation from the user buffer and the
-	 user buffer can be reused without data corruption issues.
-
-	 Since no further write can occur while we're still having ongoing
-	 I/O, this should be reasanably safe.
-
-	 Note: We only have proof that this problem actually occurs on Wine
-	 yet.  However, the MSDN language indicates that this may be a real
-	 problem on real Windows as well. */
-      if (is_nonblocking ())
-	{
-	  if (!atomic_write_buf)
-	    atomic_write_buf = cmalloc_abort (HEAP_BUF, max_atomic_write);
-	  ptr = memcpy (atomic_write_buf, ptr, chunk);
-	}
-
-      nbytes = 0;
-      DWORD nbytes_now = 0;
-      /* Write to fd in smaller chunks, accumulating a total.
-	 If there's an error, just return the accumulated total
-	 unless the first write fails, in which case return value
-	 from wait_overlapped(). */
-      while (nbytes < len)
-	{
-	  size_t left = len - nbytes;
-	  size_t len1;
-	  if (left > chunk)
-	    len1 = chunk;
-	  else
-	    len1 = left;
-	  bool res = WriteFile (get_output_handle (), ptr, len1, &nbytes_now,
-				get_overlapped ());
-	  switch (wait_overlapped (res, true, &nbytes_now,
-				   is_nonblocking (), len1))
-	    {
-	    case overlapped_success:
-	      ptr = ((char *) ptr) + chunk;
-	      nbytes += nbytes_now;
-	      break;
-	    case overlapped_error:
-	      len = 0;		/* terminate loop */
-	    case overlapped_unknown:
-	    case overlapped_nullread:
-	    case overlapped_nonblocking_no_data:
-	      break;
-	    }
-	}
-      if (!nbytes)
-	nbytes = (nbytes_now == (DWORD) -1) ? (size_t) -1 : nbytes_now;
-    }
-  return nbytes;
-}
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 7aa01d101..55b49bced 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -162,16 +162,12 @@ class fhandler_base
     unsigned need_fork_fixup    : 1; /* Set if need to fixup after fork. */
     unsigned isclosed		: 1; /* Set when fhandler is closed. */
     unsigned mandatory_locking	: 1; /* Windows mandatory locking */
-    unsigned was_nonblocking	: 1; /* Set when setting O_NONBLOCK.  Never
-					reset.  This is for the sake of
-					fhandler_base_overlapped::close. */
 
    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), isclosed (0), mandatory_locking (0),
-      was_nonblocking (0)
+      need_fork_fixup (0), isclosed (0), mandatory_locking (0)
       {}
   } status, open_status;
 
@@ -268,7 +264,6 @@ class fhandler_base
   IMPLEMENT_STATUS_FLAG (bool, need_fork_fixup)
   IMPLEMENT_STATUS_FLAG (bool, isclosed)
   IMPLEMENT_STATUS_FLAG (bool, mandatory_locking)
-  IMPLEMENT_STATUS_FLAG (bool, was_nonblocking)
 
   int get_default_fmode (int flags);
 
@@ -1119,74 +1114,6 @@ class fhandler_socket_unix : public fhandler_socket
 
 #endif /* __WITH_AF_UNIX */
 
-class fhandler_base_overlapped: public fhandler_base
-{
-  static HANDLE asio_done;
-  static LONG asio_close_counter;
-protected:
-  enum wait_return
-  {
-    overlapped_unknown = 0,
-    overlapped_success,
-    overlapped_nonblocking_no_data,
-    overlapped_nullread,
-    overlapped_error
-  };
-  bool io_pending;
-  OVERLAPPED io_status;
-  OVERLAPPED *overlapped;
-  size_t max_atomic_write;
-  void *atomic_write_buf;
-public:
-  wait_return __reg3 wait_overlapped (bool, bool, DWORD *, bool, DWORD = 0);
-  int __reg1 setup_overlapped ();
-  void __reg1 destroy_overlapped ();
-  virtual void __reg3 raw_read (void *ptr, size_t& len);
-  virtual ssize_t __reg3 raw_write (const void *ptr, size_t len);
-  OVERLAPPED *&get_overlapped () {return overlapped;}
-  OVERLAPPED *get_overlapped_buffer () {return &io_status;}
-  void set_overlapped (OVERLAPPED *ov) {overlapped = ov;}
-  fhandler_base_overlapped (): io_pending (false), overlapped (NULL), max_atomic_write (0), atomic_write_buf (NULL)
-  {
-    memset (&io_status, 0, sizeof io_status);
-  }
-  bool __reg1 has_ongoing_io ();
-
-  void fixup_after_fork (HANDLE);
-  void fixup_after_exec ();
-
-  int close ();
-  int dup (fhandler_base *child, int);
-
-  void check_later ();
-  static void __reg1 flush_all_async_io ();;
-
-  fhandler_base_overlapped (void *) {}
-  ~fhandler_base_overlapped ()
-  {
-    if (atomic_write_buf)
-      cfree (atomic_write_buf);
-  }
-
-  virtual void copyto (fhandler_base *x)
-  {
-    x->pc.free_strings ();
-    *reinterpret_cast<fhandler_base_overlapped *> (x) = *this;
-    reinterpret_cast<fhandler_base_overlapped *> (x)->atomic_write_buf = NULL;
-    x->reset (this);
-  }
-
-  virtual fhandler_base_overlapped *clone (cygheap_types malloc_type = HEAP_FHANDLER)
-  {
-    void *ptr = (void *) ccalloc (malloc_type, 1, sizeof (fhandler_base_overlapped));
-    fhandler_base_overlapped *fh = new (ptr) fhandler_base_overlapped (ptr);
-    copyto (fh);
-    return fh;
-  }
-
-  friend DWORD WINAPI flush_async_io (void *);
-};
-
 class fhandler_pipe: public fhandler_base
 {
 private:
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index a914ae8a9..634b8ba0e 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -109,7 +109,6 @@ close_all_files (bool norelease)
   if (!have_execed && cygheap->ctty)
     cygheap->close_ctty ();
 
-  fhandler_base_overlapped::flush_all_async_io ();
   if (h)
     SetStdHandle (STD_ERROR_HANDLE, h);
   cygheap->fdtab.unlock ();
-- 
2.21.0