EnterCriticalSection() is corrupting my heap

[Arsalan Ahmad]

Hi all,

I have developed a static library which I am using in one of my application.
In my library I have created my own heap and all the objects (class objects)
in my application are created in that heap. What I have observed is that in
my library at a certain place when I call EnterCriticalSection() to an
object allocated at my heap, it is corrupting my heap. I am using Windows XP
and visual studio 8.0. Any hint how can I solve this problem?

Thanks,

Arsalan

[Oleg Starodumov]

Post by Arsalan Ahmad
I have developed a static library which I am using in one of my application.
In my library I have created my own heap and all the objects (class objects)
in my application are created in that heap. What I have observed is that in
my library at a certain place when I call EnterCriticalSection() to an
object allocated at my heap, it is corrupting my heap. I am using Windows XP
and visual studio 8.0. Any hint how can I solve this problem?

How exactly do you create and use the heap?
Do you use HeapCreate/HeapAlloc/etc., or some other approach?
How do you detect the heap corruption?
How do you allocate memory for the CRITICAL_SECTION structure?
How do you pass this CRITICAL_SECTION to EnterCriticalSection?

Code samples would be helpful.

Regards,
Oleg
[VC++ MVP http://www.debuginfo.com/]

[Arsalan Ahmad]

Hi,

Yes i use HeapCreate() and HeapAlloc().

I have a class object which is created on the heap and I have a member
variable in this class of type CRITICALSECTION (say m_cs). Inside one of my
class function when I call EnterCriticalSection(&m_cs) then this problem
occurs. Ok may be its not because of critical section because at the place
in code where EnterCriticalSection() was being called I create and
CAutoLock() object and pass my pointer to CRITICALSECTION object to it
(CAutoLock just call EnterCriticalSection in its constructor and
LeaveCriticalSection in its destructor). In the constructor when I try to
save pointer of critical section to the class member (CRITICALSECTION
*m_pCS) of CAutoLock then although it is pointer assignment but after
assignement the class member has some garbage data.

In my outside code:

{
CAutoLock(&m_cs);

// Some code
}

CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; <= This assignement is not working correctly and after
assignment m_pCS points to some garbage memory location
EnterCriticalSection(m_pCS);
}

So any idea what is wrong?

Thanks,

Arsalan




Any idea whats wrong?

Post by Oleg Starodumov

Post by Arsalan Ahmad
I have developed a static library which I am using in one of my application.
In my library I have created my own heap and all the objects (class objects)
in my application are created in that heap. What I have observed is that in
my library at a certain place when I call EnterCriticalSection() to an
object allocated at my heap, it is corrupting my heap. I am using Windows XP
and visual studio 8.0. Any hint how can I solve this problem?

How exactly do you create and use the heap?
Do you use HeapCreate/HeapAlloc/etc., or some other approach?
How do you detect the heap corruption?
How do you allocate memory for the CRITICAL_SECTION structure?
How do you pass this CRITICAL_SECTION to EnterCriticalSection?
Code samples would be helpful.
Regards,
Oleg
[VC++ MVP http://www.debuginfo.com/]

[Oleg Starodumov]

Post by Arsalan Ahmad
{
CAutoLock(&m_cs);
// Some code
}
CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; <= This assignement is not working correctly and after
assignment m_pCS points to some garbage memory location
EnterCriticalSection(m_pCS);
}
So any idea what is wrong?

There can be a problem with the way the function is called (I mean the function
that instantiates CAutoLock object). It could be that it is called via a bad object
pointer, as a result "this" pointer passed to the function contains wrong value,
and so on. The next time you reproduce the problem, take a look at the value
of "this" passed to that function, and check if it's correct.

I mean something like this:

class CObj
{
...
CRITICAL_SECTION m_cs;
void YourFunc(); // instantiates CAutoLock and passes it &m_cs
}

CObj pObj; // not initialized, for example
pObj->YourFunc(); // when it is called, "this" pointer is bad, and thus pointers to
// its data members will also be bad

Generic safety checks for heap corruptions with PageHeap would not harm too,
try to enable it as described here:
http://www.debuginfo.com/tips/userbpntdll.html

Oleg

[Arsalan Ahmad]

As far as CAutoLock is concerned, I am creating its object in stack as

Post by Oleg Starodumov

{
CAutoLock lock(&m_cs);
// Some code
}

So still no idea what is wrong.

Regards,

Arsalan

Post by Oleg Starodumov

{
CAutoLock(&m_cs);
// Some code
}
CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; <= This assignement is not working correctly and after
assignment m_pCS points to some garbage memory location
EnterCriticalSection(m_pCS);
}
So any idea what is wrong?

There can be a problem with the way the function is called (I mean the function
that instantiates CAutoLock object). It could be that it is called via a bad object
pointer, as a result "this" pointer passed to the function contains wrong value,
and so on. The next time you reproduce the problem, take a look at the value
of "this" passed to that function, and check if it's correct.
class CObj
{
...
CRITICAL_SECTION m_cs;
void YourFunc(); // instantiates CAutoLock and passes it &m_cs
}
CObj pObj; // not initialized, for example
pObj->YourFunc(); // when it is called, "this" pointer is bad, and thus pointers to
// its data members will also be bad
Generic safety checks for heap corruptions with PageHeap would not harm too,
http://www.debuginfo.com/tips/userbpntdll.html
Oleg

[Oleg Starodumov]

Post by Arsalan Ahmad
As far as CAutoLock is concerned, I am creating its object in stack as

{
CAutoLock lock(&m_cs);
// Some code
}

So still no idea what is wrong.

No, I mean the function that instantiates CAutoLock. E.g. if it is:

void SomeClass::SomeFunc()
{
CAutoLock lock(&m_cs);
// Some code
}

Check "this" pointer passed to SomeClass::SomeFunc, and how
the object of SomeClass is instantiated. (E.g. use Call Stack window
to activate the previous frame on the stack, and inspect "this"
in Watch window).

Oleg

[Arsalan Ahmad]

Still no luck. Just one assignment statement inside the constructor (in
which i am assigning one pointer to another is not working) and if i dont
use CAutoLock class at all then at the same line when EnterCriticalSeciton()
statement executes another pointer gets garbage value. This is the time when
I really think about switching to .NET but I cannot :(.

Regards,

Arsalan

Post by Oleg Starodumov

Post by Arsalan Ahmad
As far as CAutoLock is concerned, I am creating its object in stack as

{
CAutoLock lock(&m_cs);
// Some code
}

So still no idea what is wrong.

void SomeClass::SomeFunc()
{
CAutoLock lock(&m_cs);
// Some code
}
Check "this" pointer passed to SomeClass::SomeFunc, and how
the object of SomeClass is instantiated. (E.g. use Call Stack window
to activate the previous frame on the stack, and inspect "this"
in Watch window).
Oleg

[Oleg Starodumov]

Post by Arsalan Ahmad
Still no luck. Just one assignment statement inside the constructor (in
which i am assigning one pointer to another is not working) and if i dont use CAutoLock class at all then at the same
line when EnterCriticalSeciton() statement executes another pointer gets garbage value. This is the time when I really
think about switching to .NET but I cannot :(.

{
CAutoLock lock(&m_cs); <== STOP HERE
// Some code
}

Enter "this" into Watch window. What value will be shown?

Then F11 (step into) CAutoLock constructor:

CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS;
EnterCriticalSection(m_pCS); <== STOP HERE
}

Enter "m_pCS" into Watch window. What value will be shown?

Oleg

[Murrgon]

You haven't called InitializeCriticalSection(&m_cs). Without
doing this, you are using an unitialized critical section.

Post by Arsalan Ahmad
Hi,
Yes i use HeapCreate() and HeapAlloc().
I have a class object which is created on the heap and I have a member
variable in this class of type CRITICALSECTION (say m_cs). Inside one of my
class function when I call EnterCriticalSection(&m_cs) then this problem
occurs. Ok may be its not because of critical section because at the place
in code where EnterCriticalSection() was being called I create and
CAutoLock() object and pass my pointer to CRITICALSECTION object to it
(CAutoLock just call EnterCriticalSection in its constructor and
LeaveCriticalSection in its destructor). In the constructor when I try to
save pointer of critical section to the class member (CRITICALSECTION
*m_pCS) of CAutoLock then although it is pointer assignment but after
assignement the class member has some garbage data.
{
CAutoLock(&m_cs);
// Some code
}
CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; <= This assignement is not working correctly and after
assignment m_pCS points to some garbage memory location
EnterCriticalSection(m_pCS);
}
So any idea what is wrong?
Thanks,
Arsalan
Any idea whats wrong?

Post by Oleg Starodumov

Post by Arsalan Ahmad
I have developed a static library which I am using in one of my application.
In my library I have created my own heap and all the objects (class objects)
in my application are created in that heap. What I have observed is that in
my library at a certain place when I call EnterCriticalSection() to an
object allocated at my heap, it is corrupting my heap. I am using Windows XP
and visual studio 8.0. Any hint how can I solve this problem?

How exactly do you create and use the heap?
Do you use HeapCreate/HeapAlloc/etc., or some other approach?
How do you detect the heap corruption?
How do you allocate memory for the CRITICAL_SECTION structure?
How do you pass this CRITICAL_SECTION to EnterCriticalSection?
Code samples would be helpful.
Regards,
Oleg
[VC++ MVP http://www.debuginfo.com/]

[Arsalan Ahmad]

Nice guess ;-) but i have called InitializeCriticalSection()...

Post by Murrgon
You haven't called InitializeCriticalSection(&m_cs). Without
doing this, you are using an unitialized critical section.

Post by Arsalan Ahmad
Hi,
Yes i use HeapCreate() and HeapAlloc().
I have a class object which is created on the heap and I have a member
variable in this class of type CRITICALSECTION (say m_cs). Inside one of
my class function when I call EnterCriticalSection(&m_cs) then this
problem occurs. Ok may be its not because of critical section because at
the place in code where EnterCriticalSection() was being called I create
and CAutoLock() object and pass my pointer to CRITICALSECTION object to
it (CAutoLock just call EnterCriticalSection in its constructor and
LeaveCriticalSection in its destructor). In the constructor when I try to
save pointer of critical section to the class member (CRITICALSECTION
*m_pCS) of CAutoLock then although it is pointer assignment but after
assignement the class member has some garbage data.
{
CAutoLock(&m_cs);
// Some code
}
CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; <= This assignement is not working correctly and after
assignment m_pCS points to some garbage memory location
EnterCriticalSection(m_pCS);
}
So any idea what is wrong?
Thanks,
Arsalan
Any idea whats wrong?

Post by Oleg Starodumov

Post by Arsalan Ahmad
I have developed a static library which I am using in one of my application.
In my library I have created my own heap and all the objects (class objects)
in my application are created in that heap. What I have observed is that in
my library at a certain place when I call EnterCriticalSection() to an
object allocated at my heap, it is corrupting my heap. I am using Windows XP
and visual studio 8.0. Any hint how can I solve this problem?

How exactly do you create and use the heap?
Do you use HeapCreate/HeapAlloc/etc., or some other approach?
How do you detect the heap corruption?
How do you allocate memory for the CRITICAL_SECTION structure?
How do you pass this CRITICAL_SECTION to EnterCriticalSection?
Code samples would be helpful.
Regards,
Oleg
[VC++ MVP http://www.debuginfo.com/]

[Norman Bullen]

Post by Arsalan Ahmad
Nice guess ;-) but i have called InitializeCriticalSection()...

Post by Murrgon
You haven't called InitializeCriticalSection(&m_cs). Without
doing this, you are using an unitialized critical section.

Post by Arsalan Ahmad
Hi,
Yes i use HeapCreate() and HeapAlloc().
I have a class object which is created on the heap and I have a member
variable in this class of type CRITICALSECTION (say m_cs). Inside one of
my class function when I call EnterCriticalSection(&m_cs) then this
problem occurs. Ok may be its not because of critical section because at
the place in code where EnterCriticalSection() was being called I create
and CAutoLock() object and pass my pointer to CRITICALSECTION object to
it (CAutoLock just call EnterCriticalSection in its constructor and
LeaveCriticalSection in its destructor). In the constructor when I try to
save pointer of critical section to the class member (CRITICALSECTION
*m_pCS) of CAutoLock then although it is pointer assignment but after
assignement the class member has some garbage data.
{
CAutoLock(&m_cs);
// Some code
}
CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; <= This assignement is not working correctly and after
assignment m_pCS points to some garbage memory location
EnterCriticalSection(m_pCS);
}
So any idea what is wrong?
Thanks,
Arsalan
Any idea whats wrong?

Post by Oleg Starodumov

Post by Arsalan Ahmad
I have developed a static library which I am using in one of my application.
In my library I have created my own heap and all the objects (class objects)
in my application are created in that heap. What I have observed is that in
my library at a certain place when I call EnterCriticalSection() to an
object allocated at my heap, it is corrupting my heap. I am using Windows XP
and visual studio 8.0. Any hint how can I solve this problem?

How exactly do you create and use the heap?
Do you use HeapCreate/HeapAlloc/etc., or some other approach?
How do you detect the heap corruption?
How do you allocate memory for the CRITICAL_SECTION structure?
How do you pass this CRITICAL_SECTION to EnterCriticalSection?
Code samples would be helpful.
Regards,
Oleg
[VC++ MVP http://www.debuginfo.com/]

Does your class have a copy constructor?

If it does, how does it initialize the critical section in the new object?

If it does not, how do you know that a default copy constructor has not
been generated? Add a _private_ copy constructor without any
implementation to the class declaration to prevent inadvertent use of a
generated copy constructor.

Norm

[Doug Harrison [MVP]]

On Thu, 25 May 2006 03:13:31 GMT, Norman Bullen

Post by Norman Bullen
Does your class have a copy constructor?
If it does, how does it initialize the critical section in the new object?
If it does not, how do you know that a default copy constructor has not
been generated? Add a _private_ copy constructor without any
implementation to the class declaration to prevent inadvertent use of a
generated copy constructor.

This is very good advice. The default should be to not support copying, and
unfortunately, you have to enforce this yourself. Unless I consciously
decide a class should support copying, I start it off like this:

class X
{
private:

// Copyguard
X(const X&);
void operator=(const X&);
};

[Arsalan Ahmad]

Why I dont need copy constructor is because of the fact that I am passing
pointer to CRITICAL_SECTION object and not the object itself;

CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; // CRITICAL_SECTION *m_pCS
EnterCriticalSection(m_pCS);
}

Post by Doug Harrison [MVP]
On Thu, 25 May 2006 03:13:31 GMT, Norman Bullen

Post by Norman Bullen
Does your class have a copy constructor?
If it does, how does it initialize the critical section in the new object?
If it does not, how do you know that a default copy constructor has not
been generated? Add a _private_ copy constructor without any
implementation to the class declaration to prevent inadvertent use of a
generated copy constructor.

This is very good advice. The default should be to not support copying, and
unfortunately, you have to enforce this yourself. Unless I consciously
class X
{
// Copyguard
X(const X&);
void operator=(const X&);
};
--
Doug Harrison
Visual C++ MVP

[Doug Harrison [MVP]]

Post by Arsalan Ahmad
Why I dont need copy constructor is because of the fact that I am passing
pointer to CRITICAL_SECTION object and not the object itself;
CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; // CRITICAL_SECTION *m_pCS
EnterCriticalSection(m_pCS);
}

I believe Norman and I were talking about the class that contains m_cs, per

Post by Arsalan Ahmad

{
CAutoLock lock(&m_cs);
// Some code
}

If you are copying objects of this class that contains m_cs via the default
copy ctor or assignment operator, you are copying a CRITICAL_SECTION
object, and I'm skeptical that's valid.

[Arsalan Ahmad]

Well i think we have moved to some off-topic thing related to copy
constructor.

Even if I dont use CAutoLock() class and manually call;

EnterCriticalSection(&cs);

// My code here

LeaveCriticalSection(&m_cs);

Even in this case calling EnterCriticalSection() is actually corrupting
another pointer in memory which has nothing to do with m_cs. So any idea
what are possible reasons because of which a heap could be corrupted like
this.

Arsalan

Post by Doug Harrison [MVP]

Post by Arsalan Ahmad
Why I dont need copy constructor is because of the fact that I am passing
pointer to CRITICAL_SECTION object and not the object itself;
CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; // CRITICAL_SECTION *m_pCS
EnterCriticalSection(m_pCS);
}

I believe Norman and I were talking about the class that contains m_cs, per

Post by Arsalan Ahmad

{
CAutoLock lock(&m_cs);
// Some code
}

If you are copying objects of this class that contains m_cs via the default
copy ctor or assignment operator, you are copying a CRITICAL_SECTION
object, and I'm skeptical that's valid.
--
Doug Harrison
Visual C++ MVP

[Oleg Starodumov]

Post by Arsalan Ahmad
Well i think we have moved to some off-topic thing related to copy
constructor.
Even if I dont use CAutoLock() class and manually call;
EnterCriticalSection(&cs);
// My code here
LeaveCriticalSection(&m_cs);
Even in this case calling EnterCriticalSection() is actually corrupting another pointer in memory which has nothing to
do with m_cs. So any idea what are possible reasons because of which a heap could be corrupted like this.
CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; <= This assignement is not working correctly and after assignment m_pCS points to some garbage
memory location
EnterCriticalSection(m_pCS);
}

Is it really true that, after the assignment, m_pCS points to "some garbage
memory location"? If it is true, the reason of the problem is _before_
EnterCriticalSection is called, and therefore EnterCriticalSection is only
a victim of some other problem (and that problem is _not_ related
to the contents of CRITICAL_SECTION structure, be it initialized or not).
And if so, could you please do the test (in the debugger) that I asked you to do
in the other branch of this discussion?

Oleg

[Stephen Kellett]

Post by Arsalan Ahmad
CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; <= This assignement is not working correctly and after
assignment m_pCS points to some garbage memory location
EnterCriticalSection(m_pCS);
}

Do you call InitializeCriticalSection() anywhere? You must call this
once before you try entering the critical section. Doesn't look to me
like you are using MFC's CCriticalSection wrapper, so I assume you are
using the raw Win32 object. Hence you need to initialize it.

VOID InitializeCriticalSection(
LPCRITICAL_SECTION lpCriticalSection // critical section
);

Thread Validator from Software Verification would have identified this
error if you had run your code through it.

http://www.softwareverify.com/threadValidator/index.html

Stephen

[Doug Harrison [MVP]]

Post by Arsalan Ahmad
Hi,
Yes i use HeapCreate() and HeapAlloc().
I have a class object which is created on the heap and I have a member
variable in this class of type CRITICALSECTION (say m_cs). Inside one of my
class function when I call EnterCriticalSection(&m_cs) then this problem
occurs. Ok may be its not because of critical section because at the place
in code where EnterCriticalSection() was being called I create and
CAutoLock() object and pass my pointer to CRITICALSECTION object to it
(CAutoLock just call EnterCriticalSection in its constructor and
LeaveCriticalSection in its destructor). In the constructor when I try to
save pointer of critical section to the class member (CRITICALSECTION
*m_pCS) of CAutoLock then although it is pointer assignment but after
assignement the class member has some garbage data.
{
CAutoLock(&m_cs);
// Some code
}
CAutoLock::CAutoLock(CRITICALSECTION *pCS)
{
m_pCS = pCS; <= This assignement is not working correctly and after
assignment m_pCS points to some garbage memory location
EnterCriticalSection(m_pCS);
}
So any idea what is wrong?

I see you corrected the above in a subsequent message, but it's worth
mentioning that the following will indeed compile, but it won't do what you
want:

{
CAutoLock(&m_cs);

// Some code
}

This will just create a temporary CAutoLock object and immediately destroy
it, so the code which follows it in the block will not execute in a
critical section.