Warrior_EA/System/AtomicFile.mqh

105 lines
6.2 KiB
MQL5
Raw Permalink Normal View History

fix: make sidecar writes atomic; extract shared AtomicFile helper FileOpen(FILE_WRITE) truncates its target on open. CNet::Save already staged the .nnw through a temp file + rename for that reason, but the three sidecars written beside it did not: .stats ExpertSignalAIBase.mqh:5918 .arrows ExpertSignalAIBase.mqh:6224 .cfg ExpertSignalAIBase.mqh:7329 Two defects followed. 1. An interrupted write published a truncated sidecar. For .cfg that is the worst case: LoadAndCompareTopologyConfiguration() reads a short file as a mismatch, which discards the trained model and restarts from era 0. 2. Windows file sharing is a mutual contract - a writer opened with no FILE_SHARE_* blocks every concurrent open regardless of the reader's flags. All three read paths carry FILE_SHARE_READ|FILE_SHARE_WRITE specifically so a tester agent can read them while a live chart runs; an exclusive writer on the same path defeated that. Extracted CNet::Save's proven pattern into System\AtomicFile.mqh (AtomicWriteBegin/AtomicWriteEnd) and routed all four writers through it. This also encodes the FileMove gotcha once instead of per call site: the destination location comes from FILE_COMMON inside the 4th arg, NOT inherited from the source, and getting it wrong moves the file to the wrong sandbox silently. Also fixed while in these functions: - SaveTopologyConfiguration had 13 copy-pasted 6-line error blocks that each returned WITHOUT FileClose(handle), leaking the handle on every write failure. Collapsed to one ok-chain that closes exactly once. The on-disk field order and types are unchanged (asserted during the rewrite) so existing .cfg files still load. - SaveChartSignals documented that pruning runs only after a successful write ("a failed write above leaves both the file AND the chart untouched") but never checked any write result, so a partial write still deleted the chart objects. Results are checked now, making the existing comment true. Compiles 0 errors, 0 warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-29 00:31:29 -04:00
//+------------------------------------------------------------------+
//| Warrior_EA |
//| AnimateDread |
//| |
//| Crash-safe binary file writes. |
//| |
//| WHY THIS EXISTS. FileOpen(FILE_WRITE) TRUNCATES its target the |
//| instant it opens. Writing straight to the destination therefore |
//| destroys the previous good copy before the first byte of the new |
//| one is written, so anything that interrupts the write - a force- |
//| kill, an OnDeinit that overruns MT5's deinit budget ("Abnormal |
//| termination"), a full disk - leaves a truncated file behind. On |
//| the next load that reads as corrupt, and for this EA that has |
//| meant retraining from era 0. |
//| |
//| The fix is to write to a temp file and rename it over the target |
//| only after a fully successful write. A rename on the same volume |
//| is atomic: an interrupted save leaves a junk .savetmp and the |
//| real file stays whole, so a restart always resumes the last |
//| COMPLETE save. |
//| |
//| It also fixes a second, subtler problem. Windows file sharing is |
//| a MUTUAL contract: a writer that opens with no FILE_SHARE_* flags |
//| blocks every concurrent open no matter what flags the READER |
//| passes. Every FileOpen(...FILE_READ...) in this codebase already |
//| carries FILE_SHARE_READ|FILE_SHARE_WRITE (see CopySharedFile); |
//| those flags cannot do their job while an exclusive writer holds |
//| the same path. Writing to a temp path instead means the live |
//| destination is never held open for writing at all, so concurrent |
//| readers (tester agent seeding its cache from a live chart's |
//| model) always succeed - and they can never observe a torn file, |
//| because the rename publishes it in one step. |
//| |
//| CNet::Save established this pattern for the .nnw; this header is |
//| that logic lifted out so the sidecars written next to it (.stats, |
//| .arrows, .cfg) get the identical guarantee instead of each |
//| re-implementing it - or, as was the case, not implementing it. |
//+------------------------------------------------------------------+
#ifndef WARRIOR_ATOMIC_FILE_MQH
#define WARRIOR_ATOMIC_FILE_MQH
//+------------------------------------------------------------------+
//| Open a temp file to stage an atomic binary write. |
//| |
//| finalName - the real destination, WITHOUT any temp suffix. |
//| commonFlag - FILE_COMMON, or 0 for this program's own sandbox. |
//| tmpName - receives the temp path; pass it back unchanged to |
//| AtomicWriteEnd(). |
//| |
//| Returns a handle opened FILE_BIN|FILE_WRITE, or INVALID_HANDLE. |
//| Callers must pair every successful call with AtomicWriteEnd(). |
//+------------------------------------------------------------------+
int AtomicWriteBegin(const string finalName, const int commonFlag, string &tmpName)
{
tmpName = finalName + ".savetmp";
ResetLastError();
return FileOpen(tmpName, commonFlag | FILE_BIN | FILE_WRITE);
}
//+------------------------------------------------------------------+
//| Close the staged write and either publish it or discard it. |
//| |
//| ok = false discards the temp and leaves the existing destination |
//| untouched, which is the entire point: a partial write must never |
//| replace a good file. |
//| |
//| `context` is used only for log messages (e.g. "CNet::Save"). |
//| Returns true only when the destination now holds the new content. |
//+------------------------------------------------------------------+
bool AtomicWriteEnd(const int handle, const string finalName, const string tmpName,
const int commonFlag, const bool ok, const string context)
{
if(handle != INVALID_HANDLE)
{
FileFlush(handle);
FileClose(handle);
}
//--- A failed/partial write must NEVER touch the real file - discard the temp, keep the last good copy.
if(!ok)
{
FileDelete(tmpName, commonFlag);
Print(context, ": write to ", tmpName, " failed - kept the existing ", finalName,
" intact (no atomic swap performed)");
return false;
}
//--- Atomic swap: FILE_REWRITE lets FileMove replace an existing destination in one operation.
//--- CRITICAL: FileMove's SOURCE location is the 2nd arg (commonFlag); the DESTINATION location is set by
//--- FILE_COMMON *inside the 4th arg* (mode_flags) - it is NOT inherited from the source. Passing only
//--- FILE_REWRITE (a real, previously-shipped bug) moved the temp from Common\Files to LOCAL\Files on a
//--- live chart (common=true): FileMove returned true so no error showed, but the file landed in the
//--- wrong sandbox and the loader (which reads Common) saw "no saved model" -> era 0 every restart.
//--- commonFlag MUST be OR'd into the destination flags so source and destination are the same location.
ResetLastError();
if(!FileMove(tmpName, commonFlag, finalName, commonFlag | FILE_REWRITE))
{
Print(context, ": atomic rename ", tmpName, " -> ", finalName, " failed (error ",
IntegerToString(GetLastError()), ") - the temp holds the new content and the existing ",
finalName, " is unchanged");
ResetLastError();
return false;
}
return true;
}
//+------------------------------------------------------------------+
#endif // WARRIOR_ATOMIC_FILE_MQH