-1

I added a new class to a dll which is part of solution. All was working fine until I started getting error discussed in this question when I would start the solution with debugger. This means that debugger was catching something bad, probably heap corruption although it was not pointing it out exactly where.

I narrowed down the issue to my added class, if I remove it, the solution debugs fine, no errors. But apparently, there is nothing wrong with the class I added!

So I commented my class and added a new simple (dummy) class Book just to see if it will produce the same error but it don't! Then I made it look similar to my added class (derived from ofstream) and still it will debug fine, no errors.

I commented it out and uncommented my original class, again the error appears when I debug it! I skimmed this class and left only constructor/destructor now I can debug it, the error goes away here too. I brought back delete code (using undo so to restored exactly) and this time it works (when it was not before) and will debug fine without throwing that corruption error!

So this sounds a lot like undefined behavior but this is a very lightweight standalone class which is instantiated correctly so is undefined behavior still the suspect? If so, why would it cause this behavior in this case?

Here are my .h and .cpp files for reference

class lfstream : public std::ofstream
{
public:
    lfstream();
    ~lfstream();

    void log(const std::string &text, int threadID);
};

#ifdef _LOGDLL
    #define API __declspec(dllexport)
#else
    #define API __declspec(dllimport)
#endif

API extern lfstream logstream;

/*
class Book : public std::ofstream
{
public: 
    Book();
    ~Book();
    int WordCount();
};

API extern Book book;
*/

The .cpp file

static char logfname[] = "debug.txt";

lfstream::lfstream(): std::ofstream( logfname )
{
}

lfstream::~lfstream()
{
}

void lfstream::log(const std::string &text, int threadID)
{
    const time_t ctt = time(0);
    *this << std::setw(40) << std::left << text << " thread id = " << threadID << "\t" << asctime(localtime(&ctt)); // << std::endl;
}

lfstream logstream;

/*
Book::Book() : std::ofstream("crash.txt")
{
}
Book::~Book()
{
}

int Book::WordCount()
{
    return 50;
}

Book book;
*/
zar
  • 11,361
  • 14
  • 96
  • 178
  • 1
    So what the error and which line is causing it? Also there are several problems with your code: 1) global variable 2) attempts to write into log from different threads (`threadID` is there for a reason, right?) without proper sync 3) attempts to use `localtime` from different threads may get you into trouble as well Also given code snippets are far from [mcve](https://stackoverflow.com/help/mcve) – user7860670 Oct 04 '17 at 21:13
  • @VTT It's a crash dialog of sort which says `windows has triggered a breakpoint in theApp.exe`. It doesn't say what happened and where but this happens in early stages of loading the executable. I have linked a post related to it in my OP. – zar Oct 04 '17 at 21:52
  • "windows has triggered a breakpoint in theApp.exe" - that might mean you have an actual function triggering a breakpoint (like, e.g. `asm { int $3 }` on a x86 system). – sehe Oct 04 '17 at 21:59
  • If you press "Break" then VS will show you the line at which an error occurred with call stack and you will be able to inspect variables, etc. – user7860670 Oct 05 '17 at 04:23
  • This isn’t the problem, but names that begin with an underscore followed by a capital letter (`_LOGDLL`) and names that contain two consecutive underscores are reserved for use by the implementation. Don’t use them in your code. (Unless you’re doing implementation-specific things with documented names like `__declspec`). – Pete Becker Oct 05 '17 at 10:53
  • @VTT pressing break lands me into assembly code, almost like it's breaking before main() and there is no meaningful call stack. No call in my project code. – zar Oct 05 '17 at 13:54

1 Answers1

2

Your code shows you are expecting to be called on different threads.

Nowhere in the code do we see thread synchronization or critical sections.

You need to protect against race-conditions by using critical sections. Having a race condition is Undefined Behaviour.

The most common standard C++ solution is using a std::mutex with std::lock_guard to make sure a single thread can be in (a) critical sections of your code.

Sample/idea:

#include <mutex>
// ....


static std::mutex s_log_mutex;

void lfstream::log(const std::string &text, int threadID)
{
    std::lock_guard<std::mutex> lock(s_log_mutex);
    const time_t ctt = time(0);
    *this << std::setw(40) << std::left << text << " thread id = " << threadID << "\t" << asctime(localtime(&ctt)); // << std::endl;
}
sehe
  • 374,641
  • 47
  • 450
  • 633
  • I did had a critical section but removed it as part of stripping it down. I started getting this error as I was developing this log module which is work in progress. As for current test cases, it does run only in single thread though but this issue start to popup and coming in the way. – zar Oct 04 '17 at 21:19
  • If you have UB and it's not multithreaded, I can assure you the origin of the UB is not in the code shown. It could be due to subtle linking issues, but judging from your descriptions that's highly unlikely at this scale. (Nonetheless, make sure you do a full clean/rebuild.) – sehe Oct 04 '17 at 21:22
  • Since the log stream is global, are you trying to log from inside constructors/destructors of other global data? You might run into init/shutdown order issues (see [SIOF](https://isocpp.org/wiki/faq/ctors#static-init-order)) – sehe Oct 04 '17 at 21:24
  • I did call it in one class's constructor, I will follow up on your link. The idea is whenever I need log, I simply call `logstream.log()` and I obviously plan to add more log methods to the class like that. – zar Oct 04 '17 at 21:27
  • The SIOF explanation makes sense and I was calling it a constructor of another class although that class itself doesn't have any static member but it could be including another class which might have. That said, I had removed ALL usage of this class, cleaned the project and I was still getting this error. The question that I posted was in fact in the same state, the class is not being used at all in solution. – zar Oct 04 '17 at 21:39
  • Also I can assure you I did remove `static` in this class as part of many trials to make it and provided hardcoded file name to constructor even though I didn't know about SIOF fiasco but +1 for that. – zar Oct 04 '17 at 21:47