SocketPosix: Close socket before freeing write buffer.

SocketPosix::Close() was freeing the write buffer before destroying a
socket.  Unclear if this is the cause of the linked bug, but it seems
not a great thing to be doing.

Bug: 1005042
Change-Id: I2cc35154b6f9de8d272244456eeed5fb9465da4a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1810321Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697718}

net/socket/socket_posix.cc

@@ -130,7 +130,10 @@ int SocketPosix::AdoptUnconnectedSocket(SocketDescriptor socket) {
 }
 
 SocketDescriptor SocketPosix::ReleaseConnectedSocket() {
-  StopWatchingAndCleanUp();
+  // It's not safe to release a socket with a pending write.
+  DCHECK(!write_buf_);
+
+  StopWatchingAndCleanUp(false /* close_socket */);
   SocketDescriptor socket_fd = socket_fd_;
   socket_fd_ = kInvalidSocket;
   return socket_fd;
@@ -400,13 +403,7 @@ bool SocketPosix::HasPeerAddress() const {
 void SocketPosix::Close() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  StopWatchingAndCleanUp();
-
-  if (socket_fd_ != kInvalidSocket) {
-    if (IGNORE_EINTR(close(socket_fd_)) < 0)
-      PLOG(ERROR) << "close() failed";
-    socket_fd_ = kInvalidSocket;
-  }
+  StopWatchingAndCleanUp(true /* close_socket */);
 }
 
 void SocketPosix::DetachFromThread() {
@@ -544,7 +541,7 @@ void SocketPosix::WriteCompleted() {
   std::move(write_callback_).Run(rv);
 }
 
-void SocketPosix::StopWatchingAndCleanUp() {
+void SocketPosix::StopWatchingAndCleanUp(bool close_socket) {
   bool ok = accept_socket_watcher_.StopWatchingFileDescriptor();
   DCHECK(ok);
   ok = read_socket_watcher_.StopWatchingFileDescriptor();
@@ -552,6 +549,16 @@ void SocketPosix::StopWatchingAndCleanUp() {
   ok = write_socket_watcher_.StopWatchingFileDescriptor();
   DCHECK(ok);
 
+  // These needs to be done after the StopWatchingFileDescriptor() calls, but
+  // before deleting the write buffer.
+  if (close_socket) {
+    if (socket_fd_ != kInvalidSocket) {
+      if (IGNORE_EINTR(close(socket_fd_)) < 0)
+        DPLOG(ERROR) << "close() failed";
+      socket_fd_ = kInvalidSocket;
+    }
+  }
+
   if (!accept_callback_.is_null()) {
     accept_socket_ = NULL;
     accept_callback_.Reset();

net/socket/socket_posix.h

@@ -44,7 +44,8 @@ class NET_EXPORT_PRIVATE SocketPosix
   // to be accepted, but must not be actually connected.
   int AdoptUnconnectedSocket(SocketDescriptor socket);
 
-  // Releases ownership of |socket_fd_| to caller.
+  // Releases ownership of |socket_fd_| to caller. There must be no pending
+  // write.
   SocketDescriptor ReleaseConnectedSocket();
 
   int Bind(const SockaddrStorage& address);
@@ -119,7 +120,8 @@ class NET_EXPORT_PRIVATE SocketPosix
   int DoWrite(IOBuffer* buf, int buf_len);
   void WriteCompleted();
 
-  void StopWatchingAndCleanUp();
+  // |close_socket| indicates whether the socket should also be closed.
+  void StopWatchingAndCleanUp(bool close_socket);
 
   SocketDescriptor socket_fd_;