Skip to content

PartFile — non-atomic part.met replacement (_tremove + _trename crash window)

Implementation

Landed in eMule-main commit 4b4087d (Harden part-file durability and disk-space handling): - ReplaceFileAtomically() added to OtherFunctions.cpp/h and routed through PartFilePersistenceSeams::TryReplaceFileAtomically(), which uses MoveFileEx(MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH). - CopyFileToTempAndReplace() added to stage .part.met.bak writes through a temp file before atomic replacement. - CPartFile::SavePartFile() now backs up the existing .part.met before replacement and promotes the temporary .part.met through ReplaceFileAtomically() instead of _tremove + _trename. - PartFileWriteThread.cpp does not own .part.met metadata promotion on current main; it writes buffered .part data and the metadata save path is centralized in CPartFile::SavePartFile(). - PartFilePersistenceSeams.h added for testability - No remaining _trename on part.met paths

Summary

The historical PartFile.cpp path saved part.met files using a two-step delete-then-rename (_tremove + _trename). If the process was killed between the two calls, the part.met file was permanently lost — no .bak fallback, no original, no temp. Fix: replace with MoveFileEx(MOVEFILE_REPLACE_EXISTING) on the same volume, and stage backup copies through a temp file before promotion.

Location

Historical srchybrid/PartFile.cpp pattern:

// after successfully writing the temporary part.met file...
if (_tremove(m_fullname) != 0 && errno != ENOENT) {
    if (thePrefs.GetVerbose())
        DebugLogError(_T("Failed to remove \"%s\""), ...);
}
if (_trename(strTmpFile, m_fullname) != 0) {
    // error logged, but m_fullname is now GONE

Current srchybrid/PartFileWriteThread.cpp: no .part.met metadata promotion path; CPartFile::SavePartFile() owns metadata persistence.

Problem

Between _tremove(m_fullname) and _trename(strTmpFile, m_fullname): - The original .part.met has been deleted - The temp file has not yet been renamed - A crash, power loss, or TerminateProcess at this exact point leaves no .part.met at all - Next startup: download file appears corrupted/incomplete even if data was intact

Additional issue: _trename does not accept \\?\ long paths — FEAT-010 (long path support) requires replacing it anyway.

Fix

Add to OtherFunctions.h/cpp:

bool ReplaceFileAtomically(const CString& strSrc, const CString& strDst, DWORD* pdwLastError = NULL)
{
    if (pdwLastError) *pdwLastError = ERROR_SUCCESS;
    if (::MoveFileEx(PreparePathForWin32LongPath(strSrc),
                     PreparePathForWin32LongPath(strDst),
                     MOVEFILE_REPLACE_EXISTING))
        return true;
    if (pdwLastError) *pdwLastError = ::GetLastError();
    return false;
}

Replace in PartFile.cpp:

// Before:
_tremove(m_fullname);
_trename(strTmpFile, m_fullname);

// After:
DWORD dwErr = ERROR_SUCCESS;
if (!ReplaceFileAtomically(strTmpFile, m_fullname, &dwErr)) {
    DebugLogError(_T("Failed to replace part.met \"%s\" - %s"), ..., GetErrorMessage(dwErr));
    ...
}

Keep .part.met metadata promotion centralized in CPartFile::SavePartFile(); PartFileWriteThread.cpp should not grow a second metadata replacement path.

For the backup copy (_fullname → _fullname.bak), add:

bool CopyFileToTempAndReplace(const CString& strSrc, const CString& strDst,
    const CString& strTmp, bool bDontOverride, DWORD* pdwLastError = NULL);

This copies to a temp location first, then atomically replaces the destination — keeping the backup write safe on low-disk conditions too.

Acceptance Criteria

  • [x] ReplaceFileAtomically added to OtherFunctions.h/cpp
  • [x] CopyFileToTempAndReplace added for backup writes
  • [x] CPartFile::SavePartFile() uses ReplaceFileAtomically
  • [x] PartFileWriteThread.cpp has no .part.met metadata promotion path on current main; metadata promotion remains centralized in CPartFile::SavePartFile()
  • [x] Backup copy uses CopyFileToTempAndReplace
  • [x] No remaining _trename on part.met paths (breaks long path support)

Evidence

  • srchybrid/OtherFunctions.h declares ReplaceFileAtomically() and CopyFileToTempAndReplace().
  • srchybrid/OtherFunctions.cpp prepares long-path-aware source, destination, and temporary paths before calling the part-file persistence seams.
  • srchybrid/PartFilePersistenceSeams.h validates null/empty paths, promotes replacements through MoveFileEx(MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH), and stages backup copies through a temporary file.
  • srchybrid/PartFile.cpp calls CopyFileToTempAndReplace() for .part.met.bak and ReplaceFileAtomically() for the temporary .part.met promotion.
  • repos/emulebb-build-tests/src/part_file_persistence.tests.cpp covers invalid replace parameters, successful atomic replacement, overlong .part.met replacement, staged backup replacement, stale temp cleanup, dont-override behavior, and preservation of live/backup content when replacement fails.
  • Static scan: rg "_trename" workspaces\v0.72a\app\eMule-main\srchybrid returns no matches on current main.