CODEWITHSUNDEEP
c++multithreadingwinapicritical-section
leon22
leon22

Reputation: 5649

File I/O issues in multithreaded code

I secure my thread function in class with CRITICAL_SECTION and do a lot of Send/Receive socket actions and everything is OK, but if threads are writing to log file I'm getting into troubles!

h

class ClassA
{
public:
    ClassA();    
    ~ClassA();

    void run();
    ...

private:
    CRITICAL_SECTION criticalSection;
    LogFiles *m_logFiles;
    ...
};

cpp

ClassA::ClassA()
{
    m_logFiles = new LogFiles();
    InitializeCriticalSection(&criticalSection);
}

ClassA::~ClassA()
{
    delete m_logFiles;
    DeleteCriticalSection(&criticalSection);
}

void ClassA::run()
{
    EnterCriticalSection(&criticalSection); 

    // do some stuff
    m_logFiles->WriteToFile(message);
    // do some stuff
    m_logFiles->WriteToFile(message);

    LeaveCriticalSection(&criticalSection);
}

Logfile contains not all information (only data from e.g. 2 of 4 threads), or overwritten lines (2 threads wrote at the same time)!

So I think I also have to secure the WriteToFile method in LogFiles?!

Thanks for any help and/or example!

Upvotes: 0

Views: 142

Answers (2)

Drew Hall
Drew Hall

Reputation: 29055

Because the CRITICAL_SECTION is an instance variable, each ClassA object effectively has its own, independent mutex. As a result, one object locking the mutex (aka entering the critical section) does not prevent any other object from doing the same to its private mutex, and the common log file resource is completely unprotected.

You can fix this by making the CRITICAL_SECTION class-static or otherwise global, and intializing it during program startup.

A better choice might be for the LogFiles class to maintain its own CRITICAL_SECTION (or even better, one for each log file that it manages), and have the WriteToFile() function explicitly lock/unlock (aka enter/leave) it internally when called. This way the resource could protect itself, rather than explicitly requiring its users to do so. It would thus always behave correctly and it would lower the burden on the users of the LogFile class.

Upvotes: 5

Ulrich Eckhardt
Ulrich Eckhardt

Reputation: 17415

Either wrap accesses to the logfile with a mutex (like win32's CRITICAL_SECTION) or use a separate thread to write logfile entries. In other words, the sketch you provide is correct as far as multithreading is concerned.

There are other big issues with your code though. Firstly, stop using pointers, in particular don't write Java/C# code like "Type* t = new Type();" all over the place. This only increases complexity and causes memory leaks. Secondly, you have to control copying and assignment of your self-defined types. Your class ClassA for example is copyable, but the instances are not independent, which leads to further subtle bugs (dangling pointers, memory leaks etc).

If fixing the C++ issues doesn't help, consider stripping down the code to a minimal example that demonstrates the problem and post it here.

Upvotes: 0

Related Questions