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;}

You at least need to show the code of AllocBundle() and isReady(). — tinman, May 10 '12 at 14:38
and of course you need to tell us (with the source code) what is going on in the background threads — Vlad, May 10 '12 at 14:43
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. — Torp, May 10 '12 at 14:49
You have several preprocessor directives in your code. Check if these code parts are properly initialized in release mode. — Andre Fly, May 10 '12 at 14:51
xcrypt: that's a lot of code. Would you do us a favour and remove all except the parts essential for the problem? — Vlad, May 10 '12 at 14:59
@Vlad 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. — xcrypt, May 10 '12 at 15:01
@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. — Vlad, May 10 '12 at 15:04
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. — tinman, May 10 '12 at 15:06
@tinman could you elaborate a bit further on that? I'm not sure what you're actually saying. — xcrypt, May 10 '12 at 15:27
@xcrypt: I was trying to say what Vlad has put in his answer. — tinman, May 10 '12 at 15:28

score 8 · Answer 1 · answered May 10 '12 at 14:44

8

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:

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

answered May 10 '12 at 14:44

John Dibling

99,718
31
186
324

1

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... – RedX May 10 '12 at 15:00
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. – John Dibling May 10 '12 at 15:02

score 5 · Answer 2 · answered May 10 '12 at 15:13

5

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.

answered May 10 '12 at 15:13

Vlad

35,022
6
77
199

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? – xcrypt May 10 '12 at 15:21
@xcrypt: No, the optimizer can cache the value in the main thread. – Vlad May 10 '12 at 15:22
1

@xcrypt: look at this article about the whole issue and importance of locking: http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf (interesting things start from 5th page) – Vlad May 10 '12 at 15:24
From what I understood, I don't explicitely need a lock, making the datamember `volatile` might be sufficient? – xcrypt May 10 '12 at 20:08
1

@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" – Vlad May 10 '12 at 20:32
@xcrypt: I would propose to try and see whether adding critical section (http://msdn.microsoft.com/en-us/library/ms885212.aspx) around will do. (You seem to be using pure WinAPI.) – Vlad May 10 '12 at 20:37
@xcrypt: sorry, wrong link, you need http://msdn.microsoft.com/en-us/library/windows/desktop/ms682608.aspx (although should be almost the same) – Vlad May 10 '12 at 20:39
sorry for bothering you again, but this: http://stackoverflow.com/questions/4557979/when-to-use-volatile-with-multi-threading 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? – xcrypt May 11 '12 at 00:48
1

@xcrypt 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.` – msam May 11 '12 at 06:20
@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. – Vlad May 11 '12 at 08:54
@msam 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 – xcrypt May 11 '12 at 12:31
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 – xcrypt May 11 '12 at 12:58
1

@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. – Vlad May 11 '12 at 14:45

score 1 · Answer 3 · answered May 10 '12 at 15:00

1

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.

answered May 10 '12 at 15:00

msam

4,259
3
19
32

It always gets there. I can see that because of the write to the console. `std::tcout<<_T("exiting allocation...")< – xcrypt May 10 '12 at 15:03

release mode error, but not in debug mode

3 Answers3

Linked