From 2e1381ec86bc794968fca51572dd7bd37d6508c7 Mon Sep 17 00:00:00 2001
From: Chris Dalton <csmartdalton@google.com>
Date: Wed, 23 Oct 2019 14:35:19 -0600
Subject: [PATCH] Don't keep "outResultsFile" open in nanobench

There is a bug on Pixel and Pixel2 devices where the program
eventually terminates with a non-zero exit code. Closing the
outResultsFile between JSON flushes seems to fix it (for whatever
reason).

Bug: b/143074513
Change-Id: I935e982e88758fda19292129c8031f8501cca615
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/249821
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
---
 bench/ResultsWriter.h        | 57 ++++++++++++++++++++++++++++++++++++
 bench/nanobench.cpp          |  4 ++-
 src/core/SkOSFile.h          |  5 ++--
 src/ports/SkOSFile_stdio.cpp |  3 ++
 4 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/bench/ResultsWriter.h b/bench/ResultsWriter.h
index f525a5df5c..bb8ff3de9a 100644
--- a/bench/ResultsWriter.h
+++ b/bench/ResultsWriter.h
@@ -12,6 +12,7 @@
 
 #include "include/core/SkString.h"
 #include "include/core/SkTypes.h"
+#include "src/core/SkOSFile.h"
 #include "src/utils/SkJSONWriter.h"
 
 /**
@@ -54,4 +55,60 @@ public:
     }
 };
 
+/**
+ NanoFILEAppendAndCloseStream: re-open the file, append the data, then close on every write() call.
+
+ The purpose of this class is to not keep the file handle open between JSON flushes. SkJSONWriter
+ uses a 32k in-memory cache already, so it only flushes occasionally and is well equipped for a
+ steam like this.
+
+ See: https://b.corp.google.com/issues/143074513
+*/
+class NanoFILEAppendAndCloseStream : public SkWStream {
+public:
+    NanoFILEAppendAndCloseStream(const char* filePath) : fFilePath(filePath) {
+        // Open the file as "write" to ensure it exists and clear any contents before we begin
+        // appending.
+        FILE* file = sk_fopen(fFilePath.c_str(), kWrite_SkFILE_Flag);
+        if (!file) {
+            SkDebugf("Failed to open file %s for write.\n", fFilePath.c_str());
+            fFilePath.reset();
+            return;
+        }
+        sk_fclose(file);
+    }
+
+    size_t bytesWritten() const override { return fBytesWritten; }
+
+    bool write(const void* buffer, size_t size) override {
+        if (fFilePath.isEmpty()) {
+            return false;
+        }
+
+        FILE* file = sk_fopen(fFilePath.c_str(), kAppend_SkFILE_Flag);
+        if (!file) {
+            SkDebugf("Failed to open file %s for append.\n", fFilePath.c_str());
+            return false;
+        }
+
+        size_t bytesWritten = sk_fwrite(buffer, size, file);
+        fBytesWritten += bytesWritten;
+        sk_fclose(file);
+
+        if (bytesWritten != size) {
+            SkDebugf("NanoFILEAppendAndCloseStream failed writing %d bytes (wrote %d instead)\n",
+                     size, bytesWritten);
+            return false;
+        }
+
+        return true;
+    }
+
+    void flush() override {}
+
+private:
+    SkString fFilePath;
+    size_t fBytesWritten = 0;
+};
+
 #endif
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index a790b31aa5..a4fe8d6cef 100644
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -1164,7 +1164,9 @@ int main(int argc, char** argv) {
     std::unique_ptr<SkWStream> logStream(new SkNullWStream);
     if (!FLAGS_outResultsFile.isEmpty()) {
 #if defined(SK_RELEASE)
-        logStream.reset(new SkFILEWStream(FLAGS_outResultsFile[0]));
+        // SkJSONWriter uses a 32k in-memory cache, so it only flushes occasionally and is well
+        // equipped for a stream that re-opens, appends, and closes the file on every write.
+        logStream.reset(new NanoFILEAppendAndCloseStream(FLAGS_outResultsFile[0]));
 #else
         SkDebugf("I'm ignoring --outResultsFile because this is a Debug build.");
         return 1;
diff --git a/src/core/SkOSFile.h b/src/core/SkOSFile.h
index 330b77bff4..082f7b8f10 100644
--- a/src/core/SkOSFile.h
+++ b/src/core/SkOSFile.h
@@ -16,8 +16,9 @@
 #include "include/core/SkString.h"
 
 enum SkFILE_Flags {
-    kRead_SkFILE_Flag   = 0x01,
-    kWrite_SkFILE_Flag  = 0x02
+    kRead_SkFILE_Flag    = 0x01,
+    kWrite_SkFILE_Flag   = 0x02,
+    kAppend_SkFILE_Flag  = 0x04
 };
 
 FILE* sk_fopen(const char path[], SkFILE_Flags);
diff --git a/src/ports/SkOSFile_stdio.cpp b/src/ports/SkOSFile_stdio.cpp
index f870c5bfd2..0e74a18970 100644
--- a/src/ports/SkOSFile_stdio.cpp
+++ b/src/ports/SkOSFile_stdio.cpp
@@ -72,7 +72,10 @@ FILE* sk_fopen(const char path[], SkFILE_Flags flags) {
         *p++ = 'r';
     }
     if (flags & kWrite_SkFILE_Flag) {
+        SkASSERT(!(flags & kAppend_SkFILE_Flag));
         *p++ = 'w';
+    } else if (flags & kAppend_SkFILE_Flag) {
+        *p++ = 'a';
     }
     *p = 'b';