release mode error, but not in debug mode

Question

My code runs fine in debug mode but fails in release mode.

Here's a snippet of my code where it fails:

LOADER->AllocBundle(&m_InitialContent);
while(!m_InitialContent.isReady())
{
    this->LoadingScreen();
}

AllocBundle() will load the content contained in m_InitialContent and set it's ready status to true when it is done. This is implemented using multithreading.

this->LoadingScreen() should render a loading screen, however at the moment that is not implemented yet so the function has an empty body.

Apparently this might be the cause of the error: If I give the function LoadingScreen() one line of code: std::cout<<"Loading"<<std::endl; then it will run fine.

If I don't, then the code gets stuck at while(!m_InitialContent.isReady()) It never even jumps to the code between the brackets (this->LoadingScreen();). And apparently neither does it update the expression in the while statement because it stays stuck there forever.

Does anyone have any ideas what might be causing this? And if so, what might the problem be? I'm completely puzzled.

---

EDIT: Additional code on request

member of ContentLoader: details::ContentBundleAllocator m_CBA;

    void ContentLoader::AllocBundle(ContentBundle* pBundle)
    {
        ASSERT(!(m_CBA.isRunning()), "ContentBundleAllocator is still busy");
        m_CBA.Alloc(pBundle, m_SystemInfo.dwNumberOfProcessors);
    }

void details::ContentBundleAllocator::Alloc(ContentBundle* pCB, UINT numThreads)
{
    m_bIsRunning = true;
    m_pCB = pCB;
    pCB->m_bIsReady = false;


    m_NumRunningThrds = numThreads;
    std::pair<UINT,HANDLE> p;
    for (UINT i = 0; i < numThreads; ++i)
    {
        p.second = (HANDLE)_beginthreadex(NULL,
                                          NULL,
                                          &details::ContentBundleAllocator::AllocBundle,
                                          this,
                                          NULL,&p.first);
        SetThreadPriority(p.second,THREAD_PRIORITY_HIGHEST);
        m_Threads.Insert(p);
    }
}

unsigned int __stdcall details::ContentBundleAllocator::AllocBundle(void* param)
{
//PREPARE
    ContentBundleAllocator* pCBA = (ContentBundleAllocator*)param;

//LOAD STUFF [collapsed for visibility+]

   //EXIT===========================================================================================================
        pCBA->m_NumRunningThrds -= 1;
        if (pCBA->m_NumRunningThrds == 0)
        {
            pCBA->m_bIsRunning = false;
            pCBA->m_pCB->m_bIsReady = true;
            pCBA->Clear();
    #ifdef DEBUG
            std::tcout << std::endl;
    #endif
            std::tcout<<_T("exiting allocation...")<<std::endl;
        }

    std::tcout<<_T("exiting thread...")<<std::endl;
    return 0;
}

bool isReady() const {return m_bIsReady;}

Lassalle

You at least need to show the code of AllocBundle() and isReady(). – Enriquetaenriquez

@Enriquetaenriquez it's quite a bite, sec – Lassalle

and of course you need to tell us (with the source code) what is going on in the background threads – Seidule

If it's some memory error it may help running it through valgrind... or whatever commercial Windows tool does the same thing. Those will catch some errors that show up only in release mode even while running in debug. – Chilon

You have several preprocessor directives in your code. Check if these code parts are properly initialized in release mode. – Disfigure

xcrypt: that's a lot of code. Would you do us a favour and remove all except the parts essential for the problem? – Seidule

@Seidule There's actually a lot more code I'm not showing that is also essential for the problem. That's why I didn't show it intially, I thought the most important part was in the small snippet above. – Lassalle

@xcrypt: I am afraid all the code dealing with textures etc is perhaps not relevant for the question. The only relevant parts are that that start/finish the threads and inform the other code about completion. – Seidule

I'm not sure whether this is correct or how to best explain it in an answer but your reading of the m_bIsReady is probably not being read from memory due to compiler optimisations. You should probably use an OS synchronisation object for that, like a manual reset event or condvar, rather than a POD and (hoping) that the threads do not optimise the access away and perform full reads and writes through the CPU caches into memory. – Enriquetaenriquez

@Seidule as you wish, I removed it. – Lassalle

@xcrypt: thanks, now it's clearer. – Seidule

@Enriquetaenriquez could you elaborate a bit further on that? I'm not sure what you're actually saying. – Lassalle

@xcrypt: I was trying to say what Vlad has put in his answer. – Enriquetaenriquez

Answer 1

When you compile your code in Debug mode, the compiler does a lot of stuff behind the scenes that prevents many mistakes made by the programmer from crashing the application. When you run in Release, all bets are off. If your code is not correct, you're much more likely to crash in Release than in Debug.

A few things to check:

1. Make sure all variables are properly intialized
2. Make sure you do not have any deadlocks or race conditions
3. Make sure you aren't passing around pointers to local objects that have been deallocated
4. Make sure your strings are properly NULL-terminated
5. Don't catch exceptions that you're not expecting and then continue running as if nothing had happened.

Canty

This has always bothered me. IMHO we should have no safe-guards on in debug so that we crash as often as possible and have safe-guards in release so that if something still slipped by we would not crash. But, i know, everything has its computational time and even the safe-guards cost performance... – Tubercle

RedX: I like developing in Debug because the safeguards are helpful sometimes. But you can develop in release, too. Just turn off optimizations, and the debugger will more or less work as expected. – Canty

Answer 2

You are accessing the variable m_bIsReady from different threads without memory barriers. This is wrong, as it may be cached by either optimizer or processor cache. You have to protect this variable from simultaneous access with a CriticalSection, or mutex, or whatever synchronization primitive is available in your library.

Note that there might be further mistakes, but this one is definitely a mistake, too. As a rule of thumb: each variable which is accessed from different threads has to be protected with a mutex/critical section/whatever.

Seidule

I don't see how this might be causing a problem? It's not guarded, no, but evetually it will reach 0 and the part m_bIsReady==0 will trigger just as intended? – Lassalle

@xcrypt: No, the optimizer can cache the value in the main thread. – Seidule

@xcrypt: look at this article about the whole issue and importance of locking: aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf (interesting things start from 5th page) – Seidule

From what I understood, I don't explicitely need a lock, making the datamember volatile might be sufficient? – Lassalle

@xcrypt: I am afraid, no, volatile is not sufficient. Some form of a memory barrier is still needed. Consider the citation from the page 10 of the article above: "though the Standard prevents compilers from reordering reads and writes to volatile data within a thread, it imposes no constraints at all on such reorderings across threads" – Seidule

@xcrypt: I would propose to try and see whether adding critical section (msdn.microsoft.com/en-us/library/ms885212.aspx) around will do. (You seem to be using pure WinAPI.) – Seidule

@xcrypt: sorry, wrong link, you need msdn.microsoft.com/en-us/library/windows/desktop/ms682608.aspx (although should be almost the same) – Seidule

sorry for bothering you again, but this: #4558479 states that volatile is actually useless for multithreading in C++. What am I supposed to believe? In the article you linked volatile is used to ensure singleton thread-safety, but this link tells me that it is useless? – Lassalle

@Lassalle from the article (end of section 5): Unfortunately, this all does nothing to address the first problem: C++’s abstract machine is single-threaded, and C++ compilers may choose to generate thread-unsafe code from source like the above, anyway. – Subnormal

@xcrypt: I second the idea that volatile is useless :( The both citations above confirm that. Although volatile may work for some compiler and compilation settings, in general it doesn't guarantee thread safety. – Seidule

@Subnormal oh yes, I've been shown that part about 10 times, but it doesn't state volatile is useless. There is nothing in the article that states something like that, only that volatile alone is not enough on multiprocessor systems. And Thanks, Vlad – Lassalle

Volatile also seems to be useful for multithreaded programming albeit not to replace memory barriers of any kind, but to indicate an object's value might change at any time. volatile bool m_bIsRunning; while (m_bIsRunning) If you don't put volatile there it might get optimised away by the compiler – Lassalle

@msam: I can bet volatile is not enough. Consider the case when the value is not flushed from the processor cache. In presence of several processors and absence of memory barriers, the different threads' view on memory is not guaranteed to be consistent. – Seidule

Answer 3

from a quick look m_NumRunningThrds doesn't seem to be protected against simultaneous access so if (pCBA->m_NumRunningThrds == 0) might never be satisfied.

Subnormal

It always gets there. I can see that because of the write to the console. std::tcout<<_T("exiting allocation...")<<std::endl; – Lassalle