[ios] Fix FD leak in IOSIntermediateDumpWriter

IOSIntermediateDumpWriter::Close() is intended to close the FD opened
by the in-process handler.

Currently, InProcessHandler::ScopedLockedWriter::~ScopedLockedWriter() does invoke IOSIntermediateDumpWriter::Close().

However, InProcessHandler::Initialize() invokes the utility CreateWriterWithPath() which directly creates an IOSIntermediateDumpWriter. It neither uses ScopedLockedWriter nor invokes Close().

This fixes the issue by:

1) Making IOSIntermediateDumpWriter::~IOSIntermediateDumpWriter() DCHECK() that it's closed
2) Calling IOSIntermediateDumpWriter::Close() from InProcessHandler::~InProcessHandler() and from test files

Change-Id: Ibfede0a3d2aeac948c7ff3d56445e13d1a4028b5
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3648710
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>

client/ios_handler/in_process_handler.cc

@@ -62,6 +62,9 @@ namespace internal {
 InProcessHandler::InProcessHandler() = default;
 
 InProcessHandler::~InProcessHandler() {
+  if (cached_writer_) {
+    cached_writer_->Close();
+  }
   UpdatePruneAndUploadThreads(false);
 }
 

client/ios_handler/in_process_intermediate_dump_handler_test.cc

@@ -49,6 +49,7 @@ class InProcessIntermediateDumpHandlerTest : public testing::Test {
   }
 
   void TearDown() override {
+    EXPECT_TRUE(writer_->Close());
     writer_.reset();
     EXPECT_FALSE(IsRegularFile(path_));
   }

snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc

@@ -61,6 +61,7 @@ class ProcessSnapshotIOSIntermediateDumpTest : public testing::Test {
   }
 
   void TearDown() override {
+    EXPECT_TRUE(writer_->Close());
     writer_.reset();
     EXPECT_FALSE(IsRegularFile(path_));
   }

util/ios/ios_intermediate_dump_reader_test.cc

@@ -53,6 +53,7 @@ class IOSIntermediateDumpReaderTest : public testing::Test {
   }
 
   void TearDown() override {
+    ASSERT_TRUE(writer_->Close());
     fd_.reset();
     writer_.reset();
     EXPECT_FALSE(IsRegularFile(path_));

util/ios/ios_intermediate_dump_writer.cc

@@ -17,6 +17,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 
+#include "base/check.h"
 #include "base/posix/eintr_wrapper.h"
 #include "build/build_config.h"
 #include "util/ios/raw_logging.h"
@@ -50,6 +51,10 @@ bool RawLoggingCloseFile(int fd) {
   return rv == 0;
 }
 
+IOSIntermediateDumpWriter::~IOSIntermediateDumpWriter() {
+  DCHECK_EQ(fd_, -1) << "Call Close() before this object is destroyed.";
+}
+
 bool IOSIntermediateDumpWriter::Open(const base::FilePath& path) {
   // Set data protection class D (No protection). A file with this type of
   // protection can be read from or written to at any time.
@@ -71,7 +76,12 @@ bool IOSIntermediateDumpWriter::Open(const base::FilePath& path) {
 }
 
 bool IOSIntermediateDumpWriter::Close() {
-  return RawLoggingCloseFile(fd_);
+  if (fd_ < 0) {
+    return true;
+  }
+  int fd = fd_;
+  fd_ = -1;
+  return RawLoggingCloseFile(fd);
 }
 
 bool IOSIntermediateDumpWriter::ArrayMapStart() {

util/ios/ios_intermediate_dump_writer.h

@@ -39,12 +39,14 @@ namespace internal {
 //! Note: All methods are `RUNS-DURING-CRASH`.
 class IOSIntermediateDumpWriter final {
  public:
-  IOSIntermediateDumpWriter() = default;
+  IOSIntermediateDumpWriter() : fd_(-1) { }
 
   IOSIntermediateDumpWriter(const IOSIntermediateDumpWriter&) = delete;
   IOSIntermediateDumpWriter& operator=(const IOSIntermediateDumpWriter&) =
       delete;
 
+  ~IOSIntermediateDumpWriter();
+
   //! \brief Command instructions for the intermediate dump reader.
   enum class CommandType : uint8_t {
     //! \brief Indicates a new map, followed by associated key.
@@ -73,6 +75,8 @@ class IOSIntermediateDumpWriter final {
   //! \brief Open and lock an intermediate dump file. This is the only method
   //!     in the writer class that is generally run outside of a crash.
   //!
+  //! The client must invoke `Close()` before this object is destroyed.
+  //!
   //! \param[in] path The path to the intermediate dump.
   //!
   //! \return On success, returns `true`, otherwise returns `false`.

util/ios/ios_intermediate_dump_writer_test.cc

@@ -41,6 +41,7 @@ class IOSIntermediateDumpWriterTest : public testing::Test {
   }
 
   void TearDown() override {
+    EXPECT_TRUE(writer_->Close());
     writer_.reset();
     EXPECT_EQ(unlink(path_.value().c_str()), 0) << ErrnoMessage("unlink");
   }