Skip to content

Race condition — shareddir_list iterated without lock in SendSharedDirectories

Implementation

Initial partial fix: eMule-main commit 4b4087d (Harden part-file durability and disk-space handling) introduced seam infrastructure: CCriticalSection CPreferences::m_csSharedDirList, CopySharedDirectoryList(), and ReplaceSharedDirectoryList().

Complete fix: eMule-main commit 0300a9d Fix shared directory list race (2026-04-09) delivers the full production fix across all mutation sites: - Preferences.cpp: CopySharedDirectoryList, ReplaceSharedDirectoryList, AddSharedDirectoryIfAbsent, IsSharedDirectoryListed all lock m_csSharedDirList — 88 lines added - BaseClient.cpp: SendSharedDirectories now snapshots via CopySharedDirectoryList before iterating - PPgDirectories.cpp: directory add/remove/load paths all use the locked helpers - SharedDirsTreeCtrl.cpp: UI tree reads via locked snapshot - SharedFileList.cpp: shared file scan uses locked snapshot - Files changed: 6 files, +88/-28 lines

Summary

BaseClient.cpp:2938-2939 iterates thePrefs.shareddir_list directly from the client request-handler thread without any lock. shareddir_list can be mutated on the UI thread (e.g., user adding/removing shared directories in preferences). This is a live race condition. eMuleAI v1.3 added a CCriticalSection m_csSharedDirList to protect all list accesses and added CopySharedDirectoryList() to make a thread-safe snapshot before iteration.

Location

srchybrid/BaseClient.cpp:2938-2939:

for (POSITION pos = thePrefs.shareddir_list.GetHeadPosition(); pos != NULL;) {
    const CString &strDir(theApp.sharedfiles->GetPseudoDirName(thePrefs.shareddir_list.GetNext(pos)));

Called from CUpDownClient::SendSharedDirectories() — this runs on the upload/download handler thread, not the UI thread.

shareddir_list is a CStringList (static CStringList shareddir_list in Preferences.h). CStringList is not thread-safe — simultaneous read + write is undefined behavior.

Race Window

The race occurs when: 1. A remote client requests our shared directory list (OP_ASKSHAREDFILES / similar) 2. Simultaneously, the user opens Preferences and modifies the shared folder list 3. The iterator pos becomes invalid mid-traversal

Outcome: access violation (crash) or empty/partial shared directory list sent to the remote peer.

Fix

In Preferences.h:

static CCriticalSection m_csSharedDirList;
static void CopySharedDirectoryList(CStringList& out);
static void ReplaceSharedDirectoryList(const CStringList& in);
static bool AddSharedDirectoryIfAbsent(const CString& dir);

In Preferences.cpp:

void CPreferences::CopySharedDirectoryList(CStringList& out)
{
    if (&out == &shareddir_list) return;
    out.RemoveAll();
    CSingleLock lock(&m_csSharedDirList, TRUE);
    for (POSITION pos = shareddir_list.GetHeadPosition(); pos != NULL;)
        out.AddTail(shareddir_list.GetNext(pos));
}

All mutation sites (add dir, remove dir, load from ini, collapse to roots) must acquire m_csSharedDirList before touching shareddir_list.

In BaseClient.cpp:2938:

// Before:
for (POSITION pos = thePrefs.shareddir_list.GetHeadPosition(); pos != NULL;) {
    const CString &strDir(theApp.sharedfiles->GetPseudoDirName(thePrefs.shareddir_list.GetNext(pos)));

// After:
CStringList sharedDirs;
thePrefs.CopySharedDirectoryList(sharedDirs);
for (POSITION pos = sharedDirs.GetHeadPosition(); pos != NULL;) {
    const CString &strDir(theApp.sharedfiles->GetPseudoDirName(sharedDirs.GetNext(pos)));

Acceptance Criteria

  • [x] CCriticalSection m_csSharedDirList added to CPreferences
  • [x] CopySharedDirectoryList() takes the lock before iterating
  • [x] All shareddir_list mutation sites acquire the lock
  • [x] BaseClient::SendSharedDirectories uses CopySharedDirectoryList before iterating
  • [x] No direct external access to shareddir_list from non-UI threads

Current Evidence

  • Preferences.h declares m_csSharedDirList and the locked shared-directory helper API.
  • Preferences.cpp defines locked copy, replace, add, remove, and membership helpers for shared directories and the related monitored-root lists.
  • Preferences.cpp::Save() snapshots the shared, monitored-root, and monitor-owned lists while holding m_csSharedDirList, then performs file I/O after releasing the lock.
  • BaseClient.cpp::SendSharedDirectories() calls thePrefs.CopySharedDirectoryList(sharedDirs) before iterating.
  • Direct shareddir_list references are now confined to Preferences.* implementation internals; external callers use the helper API or local UI copies.
  • Latest validation evidence for this workspace slice:
  • python -m emule_workspace validate
  • python -m emule_workspace build app --config Debug --platform x64
  • python -m emule_workspace test all --config Debug --platform x64