Skip to content

Fixed shared files deadlock in a multi-threaded Windows application #3210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 36 additions & 8 deletions src/utils/shared_files.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@

#include <fcntl.h>
#ifdef WIN32
#include <windows.h>
#include <io.h>
#include <algorithm>
#endif


Expand All @@ -34,7 +33,32 @@ SharedFiles::handlers_map::iterator SharedFiles::add_new_handler(
return m_handlers.end();
}

return m_handlers.insert({ fileName, {fp, 0} }).first;
#ifdef WIN32
// replace invalid characters for a Win32 named object
auto tmp = fileName;
std::replace(tmp.begin(), tmp.end(), '\\', '_');
std::replace(tmp.begin(), tmp.end(), '/', '_');

// use named mutex for multi-process locking support
const auto mutexName = "Global\\ModSecurity_" + tmp;

HANDLE hMutex = CreateMutex(NULL, FALSE, mutexName.c_str());
if (hMutex == NULL) {
error->assign("Failed to create mutex for shared file: " + fileName);
fclose(fp);
return m_handlers.end();
}
#endif

auto handler = handler_info {
fp,
#ifdef WIN32
hMutex,
#endif
0
};
// cppcheck-suppress resourceLeak ; false positive, fp is closed in SharedFiles::close
return m_handlers.insert({ fileName, handler }).first;
}


Expand Down Expand Up @@ -69,6 +93,9 @@ void SharedFiles::close(const std::string& fileName) {
if (it->second.cnt == 0)
{
fclose(it->second.fp);
#ifdef WIN32
CloseHandle(it->second.hMutex);
#endif

m_handlers.erase(it);
}
Expand All @@ -92,9 +119,11 @@ bool SharedFiles::write(const std::string& fileName,
lock.l_type = F_WRLCK;
fcntl(fileno(it->second.fp), F_SETLKW, &lock);
#else
auto handle = reinterpret_cast<HANDLE>(_get_osfhandle(fileno(it->second.fp)));
OVERLAPPED overlapped = { 0 };
::LockFileEx(handle, LOCKFILE_EXCLUSIVE_LOCK, 0, MAXDWORD, MAXDWORD, &overlapped);
DWORD dwWaitResult = WaitForSingleObject(it->second.hMutex, INFINITE);
if (dwWaitResult != WAIT_OBJECT_0) {
error->assign("couldn't lock shared file: " + fileName);
return false;
}
#endif

auto wrote = fwrite(msg.c_str(), 1, msg.size(), it->second.fp);
Expand All @@ -109,8 +138,7 @@ bool SharedFiles::write(const std::string& fileName,
lock.l_type = F_UNLCK;
fcntl(fileno(it->second.fp), F_SETLKW, &lock);
#else
overlapped = { 0 };
::UnlockFileEx(handle, 0, MAXDWORD, MAXDWORD, &overlapped);
::ReleaseMutex(it->second.hMutex);
#endif

return ret;
Expand Down
6 changes: 6 additions & 0 deletions src/utils/shared_files.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@


#include <stdio.h>
#ifdef WIN32
#include <Windows.h>
#endif

#include <unordered_map>
#include <string>
Expand Down Expand Up @@ -53,6 +56,9 @@ class SharedFiles {

struct handler_info {
FILE* fp;
#ifdef WIN32
HANDLE hMutex;
#endif
unsigned int cnt;
};

Expand Down
Loading