Thanks for this, @danim7 — really nice contribution, and I agree it's a strong foundation for the follow-ups you list (part-file verification, EC wiring, a corruption-recovery workflow, the periodic check from #166). The threading care stands out: copying the AICH hashset into a local storedFile to avoid racing a peer's OP_AICHREQUEST is exactly the right call, and the #ifndef CLIENT_GUI gating is correct here — SharedFilesCtrl.cpp is compiled per-executable, so both the menu option and the CVerifyLocalDataTask symbol are genuinely excluded from amulegui. The disk-side handling is also solid: the !file.Open guard plus the CIOFailureException/CSafeIOException catches mean a file deleted or truncated after the check is triggered degrades to a log line rather than a crash (on POSIX the open fd keeps reading the original inode anyway).

A few things from a first pass:

One blocker — files smaller than PARTSIZE (9.28 MiB). The MD4 section falls through for a single-part file that verifies OK:

if (knownFile->GetHashCount() == 0 && md4Hash != m_fileID) {
    m_corruptedMD4.push_back(part);
    break;
}
if (knownFile->GetPartHash(part) != md4Hash)   // reached even when the file is OK
    m_corruptedMD4.push_back(part);

GetPartHash is wxASSERT(part < m_hashlist.size()); return m_hashlist[part];, and m_hashlist is empty for single-part files — so an OK small file hits wxASSERT(0 < 0) in debug and an out-of-bounds read (UB) in release. A continue after the single-part case fixes it. Worth adding a sub-9.28 MiB file to your test list, since the 1 GB file is multi-part and doesn't exercise this path.

One smaller thing:

• The wxFAIL on an invalid AICH block will abort a debug build if that state is ever reachable (e.g. a last-partial-part edge). If it can happen, a debug-log-and-skip is safer than an assert; if it truly can't, a one-line comment on why would help.

Two thoughts on where this goes next:

• It'd be great to surface progress in the Shared window rather than only the log — the verify can take a while on a large file and there's no on-screen feedback today. Reusing the existing hashing/status indication (or a new state) would make it feel first-class.
• Since many people run aMule as amuled + amulegui (or amulecmd/amuleweb/amuleapi), the feature really wants to work over that path too — the EC wiring you already have on your "not in this PR" list. Whenever that lands it'll need testing end-to-end on a real remote amuled ↔ amulegui setup, not just the monolithic build.

The .po/translation and tidy items are already on your own list, so I won't pile on. Thanks again — ping me once the small-file case is handled and I'll take another pass.

Hi @got3nks thanks for the review, I will implement the changes you asked for, and nice catch on the small file case, I refactored my code too fast :D

If you have a minute, would you mind checking why the clang tidy Tier2 shows an error about undeclared wxEvent and other wx stuff in ThreadTasks.h ? I can fix it by including the <wx/event> header, but I don't understand why it fails in this job and not in the build jobs? Furthermore, the code I added in this file doesn't use those objects (not directly at least)... Maybe the Tier2 needs some tuning?

The wx errors are a red herring — they're tagged [clang-diagnostic-error], and the Tier-2 job deliberately filters those out (grep -vE 'clang-diagnostic-' in clang-tidy.yml), since the build job is the real compile gate. They don't fail the check; they're just log noise. So no need to add <wx/event.h> (though it wouldn't hurt as hygiene).

What's actually failing the gate are two maintainability findings on your new lines. Both are quick:

1. [performance-unnecessary-value-param] — PrintReport takes the CPath by value. Change just that parameter to a const reference, in both the declaration (ThreadTasks.h) and the definition (ThreadTasks.cpp):

// before
void PrintReport(const CPath fullPath, const bool checkedAICH);
// after
void PrintReport(const CPath &fullPath, const bool checkedAICH);

2. [modernize-use-emplace] — construct the pair in place instead of push_back(make_pair(...)):

// before
m_corruptedAICH.push_back(std::make_pair(part, corruptedAICHinThisPart));
// after
m_corruptedAICH.emplace_back(part, corruptedAICHinThisPart);

That's it — Tier-2 goes green after those two.

As for why the wx noise shows up only here: Tier-2 is diff-based, and your change touches the header ThreadTasks.h. Headers aren't in compile_commands.json, so when the diff tool parses the header to check your changed lines it lacks the full -I/include context a real .cpp TU has — so the header's pre-existing wxEvent/wxEventType uses (from CHashingEvent/CMediaProbeEvent, not your code) come back undeclared and it bails with "too many errors." The normal build only ever compiles that header inside ThreadTasks.cpp, where the transitive wx includes are present — which is why the build and Tier-1 stay quiet. Nothing to tune on the CI side.

Oh, thanks, I see. The output of the job is a little bit misleading: i was focusing on the lines tagged as "error" and just ignoring the "warning" lines on the first pass. Next time i will also check the "warning" from the start.

/home/runner/work/amule/amule/src/ThreadTasks.cpp:514:52: warning: the const qualified parameter 'fullPath' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]
  514 | void CVerifyLocalDataTask::PrintReport(const CPath fullPath, const bool checkedAICH)
      |                                                    ^
      |                                                   &
/home/runner/work/amule/amule/src/ThreadTasks.cpp:662:22: warning: use emplace_back instead of push_back [modernize-use-emplace]
  662 |                                         m_corruptedAICH.push_back(
      |                                                         ^~~~~~~~~~
      |                                                         emplace_back
  663 |                                                 std::make_pair(part, corruptedAICHinThisPart));
      |                                                 ~~~~~~~~~~~~~~                              ~

602 warnings generated.
1638 warnings generated.

error: too many errors emitted, stopping now [clang-diagnostic-error]
/home/runner/work/amule/amule/src/ThreadTasks.h:235:30: error: expected class name [clang-diagnostic-error]
  235 | class CHashingEvent : public wxEvent

Concerning the PR, I implemented your changes, but i will run a couple of extra tests during the weekend before marking it ready. Feel free to comment anything you see.

Nice, all four are handled cleanly:

• ✅ Small-file case — if (GetHashCount() == 0) { … break; } no longer falls through to GetPartHash.
• ✅ PrintReport(const CPath &fullPath, …).
• ✅ emplace_back(part, corruptedAICHinThisPart).
• ✅ the wxFAIL is now a AddDebugLogLineN(logVerifyLocalData, …) + continue — exactly right, that state can be hit on a partial AICH tree and shouldn't abort a debug build.

Three tiny things for whenever you finalize (none blocking, and take them or leave them):

• The // for testing: printf '\xFF' | dd … scaffolding comment near the top of CVerifyLocalDataTask can come out.
• CVerifyLocalDataTask(const CMD4Hash&) could be explicit.
• When you do the .po pass: the user-facing result lines ("Verify Local Data (%s): Result OK…", "ERRORS FOUND!…") want _() so they land in the catalog; the AddDebugLogLineN(logVerifyLocalData, …) debug lines are fine left untranslated.

Looks good otherwise.

Thanks for the review, I implemented your requested changes and I did some extra tests on my side, it shall be good now.

One thing we missed on the earlier passes, sorry — I think it's worth fixing before merge.

Entry() holds the raw CKnownFile * from FindKnownFileByID() and dereferences it throughout the hash (GetPartCount, GetPartSize, CreateHashFromFile, GetPartHash, GetAICHHashset…), which can run for minutes on a large file. This is the in-memory object, separate from the disk-file handling we already covered. Unsharing alone is safe — CSharedFileList::RemoveFile keeps the CKnownFile alive in knownfiles — but CKnownFileList::PruneDuplicates (which runs inside Save(), e.g. when another file finishes hashing) can delete it. It skips anything still in sharedfiles/downloadqueue, so a file that stays shared is safe; the gap is when the file is unshared during the verify and it's a size-duplicate and a known.met save fires in that window → the worker's pointer dangles (use-after-free). Narrow, but real.

Cleanest fix is the ownership discipline CHashingTask already uses: grab everything the loop needs in one up-front block, then never touch the shared object again. Handily CKnownFile::CreateHashFromFile() is static, and storedFile.SetFileSize() already gives you the part geometry — so on top of what you already copy for AICH, the loop just needs a snapshot of the per-part MD4 list:

CPath   fullPath;
uint64  fileSize = 0;
std::vector<CMD4Hash> partHashes;   // per-part MD4; empty for single-part files
CAICHHash aichMaster;
// + snapshot the AICH status too
{
    // (ideally under whatever guards knownfiles; either way the window
    //  is now just this small block instead of the whole hash)
    CKnownFile *kf = theApp->knownfiles->FindKnownFileByID(m_fileID);
    if (kf == nullptr || kf->IsPartFile()) return;   // log as you do now
    fullPath = kf->GetFilePath().JoinPaths(kf->GetFileName());
    fileSize = kf->GetFileSize();
    partHashes.reserve(kf->GetHashCount());
    for (size_t i = 0; i < kf->GetHashCount(); ++i)
        partHashes.push_back(kf->GetPartHash(i));
    aichMaster = kf->GetAICHHashset()->GetMasterHash();
}
// no kf below this point

CKnownFile storedFile;
storedFile.SetFileSize(fileSize);                       // -> GetPartCount()/GetPartSize()
storedFile.GetAICHHashset()->SetMasterHash(aichMaster, /*status*/);
// ...
for (uint16 part = 0; part < storedFile.GetPartCount() && !TestDestroy(); ++part) {
    const uint64 partLength = storedFile.GetPartSize(part);
    // ...
    CKnownFile::CreateHashFromFile(file, offset, partLength, &md4Hash, aichHash);  // static
    // ...
    if (partHashes.empty()) {                           // was GetHashCount() == 0
        if (md4Hash != m_fileID) m_corruptedMD4.push_back(part);
        break;
    }
    if (partHashes[part] != md4Hash) m_corruptedMD4.push_back(part);
}

Minor while you're in there: the "…not supported on PartFile" line uses CFormat("…") without _(), while the result lines are translated — worth making consistent.

Everything else still looks good — these are the only things holding it.

You are absolutely right on the possibility of a dangling pointer for the known file in case of deletion. May you please check if it is ok now?

Everything looks good now — merged. Thanks!