Home > Archive > VC Language > May 2006 > EnterCriticalSection() is corrupting my heap
You are viewing an archived Text-only version of the thread.
To view this thread in it's original format and/or if you want to reply to
this thread please [click here]
| Author |
EnterCriticalSection() is corrupting my heap
|
|
| Arsalan Ahmad 2006-05-24, 8:09 am |
| 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 2006-05-24, 8:09 am |
|
> 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 2006-05-24, 8:09 am |
| 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?
"Oleg Starodumov" <com-dot-debuginfo-at-oleg> wrote in message
news:esmFGFzfGHA.4792@TK2MSFTNGP03.phx.gbl...
>
>
> 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 2006-05-24, 8:09 am |
|
>
> 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?
>
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
| |
| Murrgon 2006-05-24, 7:10 pm |
| You haven't called InitializeCriticalSection(&m_cs). Without
doing this, you are using an unitialized critical section.
Arsalan Ahmad wrote:
> 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?
>
>
> "Oleg Starodumov" <com-dot-debuginfo-at-oleg> wrote in message
> news:esmFGFzfGHA.4792@TK2MSFTNGP03.phx.gbl...
>
>
| |
| Arsalan Ahmad 2006-05-24, 7:10 pm |
| As far as CAutoLock is concerned, I am creating its object in stack as
follows:
So still no idea what is wrong.
Regards,
Arsalan
"Oleg Starodumov" <com-dot-debuginfo-at-oleg> wrote in message
news:O6h$9lzfGHA.5092@TK2MSFTNGP04.phx.gbl...[color=darkred]
>
>
> 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
>
>
>
>
>
>
| |
| Oleg Starodumov 2006-05-24, 7:10 pm |
|
> As far as CAutoLock is concerned, I am creating its object in stack as
> follows:
>
>
> 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 2006-05-24, 7:10 pm |
| Nice guess ;-) but i have called InitializeCriticalSection()...
"Murrgon" <murrgon@hotmail.com> wrote in message
news:u49tZ9zfGHA.3900@TK2MSFTNGP05.phx.gbl...[color=darkred]
> You haven't called InitializeCriticalSection(&m_cs). Without
> doing this, you are using an unitialized critical section.
>
> Arsalan Ahmad wrote:
| |
| Arsalan Ahmad 2006-05-24, 7:10 pm |
| 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
"Oleg Starodumov" <com-dot-debuginfo-at-oleg> wrote in message
news:OBc51D0fGHA.2208@TK2MSFTNGP05.phx.gbl...
>
>
> 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
>
>
>
>
| |
| Stephen Kellett 2006-05-24, 7:10 pm |
| In message <eA$6wSzfGHA.2172@TK2MSFTNGP04.phx.gbl>, Arsalan Ahmad
<arsal__@hotmail.com> writes
>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/threa...ator/index.html
Stephen
--
Stephen Kellett
Object Media Limited http://www.objmedia.demon.co.uk/software.html
Computer Consultancy, Software Development
Windows C++, Java, Assembler, Performance Analysis, Troubleshooting
| |
| Oleg Starodumov 2006-05-24, 7:10 pm |
|
> 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 :(.
>
Run the application under debugger and stop at the following line:
[color=darkred]
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
| |
| Norman Bullen 2006-05-24, 10:06 pm |
| Arsalan Ahmad wrote:
> Nice guess ;-) but i have called InitializeCriticalSection()...
>
> "Murrgon" <murrgon@hotmail.com> wrote in message
> news:u49tZ9zfGHA.3900@TK2MSFTNGP05.phx.gbl...
>
>
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
--
--
To reply, change domain to an adult feline.
| |
| Doug Harrison [MVP] 2006-05-25, 4:19 am |
| On Thu, 25 May 2006 03:13:31 GMT, Norman Bullen
<norm@BlackKittenAssociates.com.INVALID> wrote:
>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&);
};
--
Doug Harrison
Visual C++ MVP
| |
| Doug Harrison [MVP] 2006-05-25, 4:19 am |
| On Wed, 24 May 2006 15:19:27 +0200, "Arsalan Ahmad" <arsal__@hotmail.com>
wrote:
>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?
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.
--
Doug Harrison
Visual C++ MVP
| |
| Arsalan Ahmad 2006-05-26, 8:07 am |
| 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);
}
"Doug Harrison [MVP]" <dsh@mvps.org> wrote in message
news:v5ba7297fpn54cblgvb3eh7haf2gr55ln0@
4ax.com...
> On Thu, 25 May 2006 03:13:31 GMT, Norman Bullen
> <norm@BlackKittenAssociates.com.INVALID> wrote:
>
>
> 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&);
> };
>
> --
> Doug Harrison
> Visual C++ MVP
| |
| Doug Harrison [MVP] 2006-05-26, 7:09 pm |
| On Fri, 26 May 2006 12:54:51 +0200, "Arsalan Ahmad" <arsal__@hotmail.com>
wrote:
>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
your earlier message that presented:
[color=darkred]
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
| |
| Arsalan Ahmad 2006-05-26, 7:09 pm |
| 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
"Doug Harrison [MVP]" <dsh@mvps.org> wrote in message
news:s07e72dp0dkp42288lm4h7fpg2i7crs6u9@
4ax.com...
> On Fri, 26 May 2006 12:54:51 +0200, "Arsalan Ahmad" <arsal__@hotmail.com>
> wrote:
>
>
> I believe Norman and I were talking about the class that contains m_cs,
> per
> your earlier message that presented:
>
>
> 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 2006-05-26, 7:09 pm |
|
> 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.
>
Remember one of your previous statements:
> 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
|
|
|
|
|