Disconnect native messaging port on read pipe EOF.

chrome/browser/extensions/api/messaging/native_message_process_host.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/api/messaging/native_message_process_host.h"
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/platform_file.h"
@@ skipping 19 matching lines @@
 
 // Message header contains 4-byte integer size of the message.
 const size_t kMessageHeaderSize = 4;
 
 // Size of the buffer to be allocated for each read.
 const size_t kReadBufferSize = 4096;
 
 const char kFailedToStartError[] = "Failed to start native messaging host.";
 const char kInvalidNameError[] =
     "Invalid native messaging host name specified.";
+const char kNativeHostExited[] = "Native host has exited.";
 const char kNotFoundError[] = "Specified native messaging host not found.";
 const char kForbiddenError[] =
     "Access to the specified native messaging host is forbidden.";
 const char kHostInputOuputError[] =
     "Error when communicating with the native messaging host.";
 
 }  // namespace
 
 namespace extensions {
 
 NativeMessageProcessHost::NativeMessageProcessHost(
     base::WeakPtr<Client> weak_client_ui,
     const std::string& source_extension_id,
     const std::string& native_host_name,
     int destination_port,
     scoped_ptr<NativeProcessLauncher> launcher)
     : weak_client_ui_(weak_client_ui),
       source_extension_id_(source_extension_id),
       native_host_name_(native_host_name),
       destination_port_(destination_port),
       launcher_(launcher.Pass()),
       closed_(false),
+      read_file_(base::kInvalidPlatformFileValue),
       read_pending_(false),
-      read_eof_(false),
       write_pending_(false) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   // It's safe to use base::Unretained() here because NativeMessagePort always
   // deletes us on the IO thread.
   content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE,
       base::Bind(&NativeMessageProcessHost::LaunchHostProcess,
                  base::Unretained(this)));
 }
 
@@ skipping 115 matching lines @@
 void NativeMessageProcessHost::OnFileCanWriteWithoutBlocking(int fd) {
   NOTREACHED();
 }
 #endif  // !defined(OS_POSIX)
 
 void NativeMessageProcessHost::ReadNowForTesting() {
   DoRead();
 }
 
 void NativeMessageProcessHost::WaitRead() {
-  if (closed_ || read_eof_)
+  if (closed_)
     return;
 
   DCHECK(!read_pending_);
 
   // On POSIX FileStream::Read() uses blocking thread pool, so it's better to
   // wait for the file to become readable before calling DoRead(). Otherwise it
   // would always be consuming one thread in the thread pool. On Windows
   // FileStream uses overlapped IO, so that optimization isn't necessary there.
 #if defined(OS_POSIX)
   base::MessageLoopForIO::current()->WatchFileDescriptor(
     read_file_, false /* persistent */, base::MessageLoopForIO::WATCH_READ,
     &read_watcher_, this);
 #else  // defined(OS_POSIX)
   DoRead();
 #endif  // defined(!OS_POSIX)
 }
 
 void NativeMessageProcessHost::DoRead() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
-  while (!closed_ && !read_eof_ && !read_pending_) {
+  while (!closed_ && !read_pending_) {
     read_buffer_ = new net::IOBuffer(kReadBufferSize);
     int result = read_stream_->Read(
         read_buffer_.get(),
         kReadBufferSize,
         base::Bind(&NativeMessageProcessHost::OnRead, base::Unretained(this)));
     HandleReadResult(result);
   }
 }
 
 void NativeMessageProcessHost::OnRead(int result) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
   DCHECK(read_pending_);
   read_pending_ = false;
 
   HandleReadResult(result);
   WaitRead();
 }
 
 void NativeMessageProcessHost::HandleReadResult(int result) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
   if (closed_)
     return;
 
   if (result > 0) {
     ProcessIncomingData(read_buffer_->data(), result);
-  } else if (result == 0) {
-    read_eof_ = true;
   } else if (result == net::ERR_IO_PENDING) {
     read_pending_ = true;
+  } else if (result == 0 || result == net::ERR_CONNECTION_RESET) {

[wtc, 2013/09/25 23:03:12]

On Unix, EPIPE is an error code for write/send. Here we are handling read
result. Can we get net::ERR_CONNECTION_RESET from read on Windows?

[akalin, 2013/09/25 23:09:32]

I'm guessing that we're getting ERROR_BROKEN_PIPE on read on Windows, and so the
new mapping will then generate this case.

[Sergey Ulanov, 2013/09/25 23:30:51]

On windows we always get ERROR_BROKEN_PIPE when reading from a broken pipe. On
posix read() returns 0 in that case.

[wtc, 2013/09/26 17:14:05]

Thanks. It would be nice to add a comment about this.

I found this behavior documented in the MSDN page for ReadFile():

==========
Pipes

If an anonymous pipe is being used and the write handle has been closed, when
ReadFile attempts to read using the pipe's corresponding read handle, the
function returns FALSE and GetLastError returns ERROR_BROKEN_PIPE.

...

If the lpNumberOfBytesRead parameter is zero when ReadFile returns TRUE on a
pipe, the other end of the pipe called the WriteFile function with
nNumberOfBytesToWrite set to zero.
==========

Are you using an anonymous pipe and reading from it with ReadFile()?

If we disallow writing 0 bytes to the pipe, then we can change the
ERROR_BROKEN_PIPE error to a read byte count of 0 with no risk of confusion.
Would you be willing to do that?

Alternatively, it may be a good idea to perform the result ==
net::ERR_CONNECTION_RESET check on Windows only, because on Unix
the EPIPE or ECONNRESET error is a sign of an abrupt (not graceful) connection
closure.

+    Close(kNativeHostExited);
   } else {
     LOG(ERROR) << "Error when reading from Native Messaging host: " << result;
     Close(kHostInputOuputError);
   }
 }
 
 void NativeMessageProcessHost::ProcessIncomingData(
     const char* data, int data_size) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
@@ skipping 80 matching lines @@
     closed_ = true;
     read_stream_.reset();
     write_stream_.reset();
     content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
         base::Bind(&Client::CloseChannel, weak_client_ui_,
                    destination_port_, error_message));
   }
 }
 
 }  // namespace extensions