EnterCriticalSection() killing thread?

[mournerslament]

Hi, I have limited experience with multithreaded applications, but the most experience on my team, so I am trying to implement it in our game we are working on. The purpose of the thread is to simply update all of the skeletal hierarchies of our characters in our game.

I have had a host of problems with it, thus far, despite all the reading I have done. My current problem may be the most frustrating: My thread is being killed somehow upon a call to EnterCriticalSection() in the thread. This is a relatively recent issue, and I do not even know how it started to do this, since I do not remember changing anything, at least not that would affect something like this.

Here is the code where the thread is running. Sorry about the formatting, I just spent 15 minutes trying to get it looking anywhere near readable, but the code tags are doing some weird stuff to my code, so I just copied and pasted it again...Is there some sort of secret to actually get it to format nicely? :



unsignedint__stdcall DR_AnimationSystem::UpdateHierarchies(void *pAnim)

{

unsignedint num = 0;

DR_AnimationSystem *pAnimSystem = (DR_AnimationSystem *)pAnim;

///bool bUpdate = true;

while(1)

{



//Gets here once, and then never gets to the next line
EnterCriticalSection(&(pAnimSystem->m_cs));

{

num = static_cast<unsignedint>(pAnimSystem->m_vHierarchies.size());


}

LeaveCriticalSection(&(pAnimSystem->m_cs));



for(unsignedint a = 0; a < num; a = a+1)

{

if(!pAnimSystem->m_vHierarchies[a])

{

EnterCriticalSection(&(pAnimSystem->m_cs));

{

pAnimSystem->m_vHierarchies.erase(pAnimSystem->m_vHierarchies.begin()+a--);

}

LeaveCriticalSection(&(pAnimSystem->m_cs));

}

else

{

pAnimSystem->m_vHierarchies[a]->Process();

}



}



Sleep(1);

}

return 0;

}





This was running fine earlier, I was just having some other problems. I don't think I changed anything to mess with this. I would post more code, but there is a lot of it, and I do not see how any of it would mess what is going on here. The other thread still runs fine, and everything works, this thread is just...gone?

I know I am not terminating it, I even took out my cleanup code in order to make sure, and I am still having this problem.

Please tell me I am making some stupid mistake somewhere. Any help would be very very appreciated, since I am on a very tight schedule right now and need to get this done soon. Again, sorry about how nasty the code looks, I really did try to format it, but I just made things worse. Thanks.

[miteshpandey]

Are you forgetting to initialize the critical section?


void InitializeCriticalSection(
LPCRITICAL_SECTION lpCriticalSection
);

[mournerslament]

No, I am initializing it. Thank you for the suggestion, though. I just changed *all* of my EnterCriticalSection() calls to TryEnterCriticalSection() calls, and it no longer terminates my thread, but now, of course, I am not getting the data that I need, apparently, because it looks like my matrices are full of crap. I am in the process of slowly changing each of them back one by one, until it stops working again.

[MikeAThon]

.... apparently, because it looks like my matrices are full of crap. .....
You have many race conditions. As one example:

for(unsignedint a = 0; a < num; a = a+1)
{
if(!pAnimSystem->m_vHierarchies[a])
{
EnterCriticalSection(&(pAnimSystem->m_cs));
{
pAnimSystem->m_vHierarchies.erase(pAnimSystem->m_vHierarchies.begin()+a--);
} // etc...
Between the time of the "if" statement where you check if m_vHierarchies[a] exists, and the time that you enter the critical section in order to protect m_vHierarchies and erase it, the content of m_vHierarchies can easily change. This is a race condition, since the outcome of the erase() function will depend on the order of execution between the two threads.

You need to do a better job on when and where you protect your shared data.

Mike

[mournerslament]

Thanks, Mike. I can't believe I didn't see that one earlier. None of the hierarchies are being removed right now, so it's not causing problems for me now, but I am sure that I probably messed up like that somewhere else.

Having to do with protecting the data...in a lot of places I am making local copies of the data I need inside of the critical section and then using the copy of the data where it is needed, and then copying it back, if needed, inside of another critical section. Is this an acceptable solution? Or is there something wrong with this method?


Thanks for the replies, guys, I really appreciate the help.

[Arjay]

Thanks, Mike. I can't believe I didn't see that one earlier. None of the hierarchies are being removed right now, so it's not causing problems for me now, but I am sure that I probably messed up like that somewhere else.

Having to do with protecting the data...in a lot of places I am making local copies of the data I need inside of the critical section and then using the copy of the data where it is needed, and then copying it back, if needed, inside of another critical section. Is this an acceptable solution? Or is there something wrong with this method?


Thanks for the replies, guys, I really appreciate the help.There are other solutions that may may be cleaner. Before I comment can you give me an idea of the type contained by m_vHierarchies? Assuming this is a std::vector, what type does the vector contain?

[mournerslament]

There are other solutions that may may be cleaner. Before I comment can you give me an idea of the type contained by m_vHierarchies? Assuming this is a std::vector, what type does the vector contain?

It is an std::vector of pointers to a custom type, DR_AnimationHierarchy. The hierarchy contains another class, an interpolator, that interpolates between the matrices for the current keyframes in an animation based on a time value. The hierarchy feeds information to the interpolator and flattens the skeletal hierarchy in the process function, so all of the matrices are in world space instead of local space. That is pretty much it. Hope this is enough information. Thanks.

[exterminator]

Having to do with protecting the data...in a lot of places I am making local copies of the data I need inside of the critical section and then using the copy of the data where it is needed, and then copying it back, if needed, inside of another critical section. Is this an acceptable solution? Or is there something wrong with this method?I think this can be a good strategy. With this you are reducing the contention for most part of the processing except for when the actual data needs to be updated. Those parts can remain lock free. But it would also depend on how much do you copy and how expensive can your "copy it back" algorithm be and if it affects something else or not.