1 files changed, 28 insertions, 10 deletions

diff --git a/src/rpc.cpp b/src/rpc.cpp
index f853e78..be265db 100644
--- a/src/rpc.cpp
+++ b/src/rpc.cpp
@@ -506,20 +506,35 @@ static void SyncDirectoryOf(const std::string& path)
 // write mutex for `path`: the temp name is derived from the target, so two
 // threads in here at once for one path is precisely the race this exists to
 // prevent.
+//
+// Every failure removes the temp file before returning. It used to be left
+// where it fell, so a disk that filled up once left a short, valid-looking
+// "pending_payouts.json.tmp" next to the good ledger for as long as the node
+// ran. That file is a trap: it has the name of a half-finished save, and an
+// operator cleaning up after an incident is exactly the person who will move
+// it into place.
+//
+// Failures print unconditionally rather than through LogPrint(). These three
+// files are written under two different debug categories ("payout" for the
+// ledgers, "pool" for the status file), and a write failure is not a trace of
+// normal operation -- it is the file the operator is about to read not being
+// there. Gating it behind a category that had to be chosen before the failure
+// happened is how "could not replace pool_status.json" went missing.
 static bool WriteFileAtomic(const std::string& path, const std::string& data,
                             const char* what)
 {
     std::string tmp = path + ".tmp";
     FILE* f = fopen(tmp.c_str(), "w");
     if (!f) {
-        LogPrint("payout", "[payout] WARNING: could not write %s (%s)\n",
-                 tmp.c_str(), what);
+        printf("[payout] WARNING: could not write %s (%s)\n",
+               tmp.c_str(), what);
         return false;
     }
     if (!data.empty() && fwrite(data.c_str(), 1, data.size(), f) != data.size()) {
-        LogPrint("payout", "[payout] WARNING: short write to %s (%s)\n",
-                 tmp.c_str(), what);
+        printf("[payout] WARNING: short write to %s (%s)\n",
+               tmp.c_str(), what);
         fclose(f);
+        remove(tmp.c_str());
         return false;
     }
     // Before the rename, not after: publishing a name that points at nothing
@@ -527,9 +542,10 @@ static bool WriteFileAtomic(const std::string& path, const std::string& data,
     // made durable, do not rename -- the previous file is still intact and
     // still correct, and the next write will try again.
     if (!SyncFileHandle(f)) {
-        LogPrint("payout", "[payout] WARNING: could not flush %s to disk (%s)"
-                 " -- keeping the previous file\n", tmp.c_str(), what);
+        printf("[payout] WARNING: could not flush %s to disk (%s)"
+               " -- keeping the previous file\n", tmp.c_str(), what);
         fclose(f);
+        remove(tmp.c_str());
         return false;
     }
     // fclose() is where buffered data actually reaches the OS, so a failure
@@ -537,8 +553,9 @@ static bool WriteFileAtomic(const std::string& path, const std::string& data,
     // destroy the good copy, which is exactly what these files exist to
     // prevent.
     if (fclose(f) != 0) {
-        LogPrint("payout", "[payout] WARNING: could not close %s (%s)\n",
-                 tmp.c_str(), what);
+        printf("[payout] WARNING: could not close %s (%s)\n",
+               tmp.c_str(), what);
+        remove(tmp.c_str());
         return false;
     }
 #ifdef _WIN32
@@ -546,8 +563,9 @@ static bool WriteFileAtomic(const std::string& path, const std::string& data,
     remove(path.c_str());
 #endif
     if (rename(tmp.c_str(), path.c_str()) != 0) {
-        LogPrint("payout", "[payout] WARNING: could not replace %s (%s)\n",
-                 path.c_str(), what);
+        printf("[payout] WARNING: could not replace %s (%s)\n",
+               path.c_str(), what);
+        remove(tmp.c_str());
         return false;
     }
     SyncDirectoryOf(path);