Board index » cppbuilder » Multi-Thread and memory allocation error (or bug in 5.02).

2003-08-30 12:10:07 AM
cppbuilder79
I am experience some problems with Borland 5.02 with multitheads and memory
allocation and deallocation. The same code on Borland Builder 6 appears to
work fine. I have included an example program which creates multiple
instances of the same class then deletes them. The class constructor creates
a new thread and allocates a large block of memory while the destructor
deallocates the memory and terminates the thread.
I get an access violation at 0x634b5e: read of address 0x4cdc058 when I run
the example. This corresponds to the line
delete[] array;
in the class destructor. If the creation and termination of the threads is
removed the example runs fine.
Does anyone spot an obvious error in my example or is this a bug in the
Run-Time Library?
Regards
Martin
#include <windows.h>
#include <process.h>
//#include <iostream.h>
//#define ASIZE 159727
#define ASIZE 159727
void _USERENTRY ThreadFunction(HANDLE hEvent)
{
WaitForSingleObject(hEvent, INFINITE);
}
class TVMImageList
{
public:
TVMImageList(unsigned long noImages, unsigned long size);
~TVMImageList();
public:
unsigned long mSize;
unsigned long mLength;
int* array;
int** thearray;
HANDLE hEvent;
HANDLE mThread;
unsigned long mThreadID;
};
TVMImageList::TVMImageList(unsigned long noImages, unsigned long size)
:mSize(size), mLength(noImages)
{
hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
mThread = (HANDLE)_beginthreadNT(
ThreadFunction, // pointer to thread function
4096, // stack size
hEvent, // argument for new thread
NULL,
0,
&mThreadID
);
SetThreadPriority((void*)mThread,THREAD_PRIORITY_NORMAL);
thearray = new int*[mLength];
for (int i =0; i<mLength;i++)
thearray[i] = new int[size];
for (int j=0; j < mLength ; j++){
array = thearray[j];
for (int i=0;i<mSize;i++)
array[i]=i;
}
}
TVMImageList::~TVMImageList()
{
SetEvent(hEvent);
WaitForSingleObject(mThread, INFINITE);
CloseHandle(hEvent);
for (int j=0; j < mLength ; j++){
array = thearray[j];
for (int i=0;i<mSize;i++)
if(array[i]!=i){};
// cout << "Error";
}
for (int j=0; j < mLength ; j++){
array = thearray[j];
delete[] array;
}
delete[] thearray;
}
void main()
{
TVMImageList* list = new TVMImageList(10, ASIZE);
TVMImageList* list1 = new TVMImageList(10, ASIZE);
TVMImageList* list2 = new TVMImageList(10, ASIZE);
TVMImageList* list3 = new TVMImageList(10, ASIZE);
TVMImageList* list4 = new TVMImageList(10, ASIZE);
TVMImageList* list5 = new TVMImageList(10, ASIZE);
TVMImageList* list6 = new TVMImageList(10, ASIZE);
TVMImageList* list7 = new TVMImageList(10, ASIZE);
delete list7;
delete list6;
delete list5;
delete list4;
delete list3;
delete list2;
delete list1;
delete list;
// delete list8;
}
 

Martin Catney wrote:
I would think you should create the arrays before you start the
thread.
And I would change this
while( mLength-- ) //mLength no longer needed
delete[] theArray[ mLength ]; // reverse order of new
I would also make "array" a local variable instead of an object
member. At least, the code you gave doesn't require a member named
"array".
Don't know if any of that will fix your problem, but I'd start there.
 

I see nothing obviously wrong in the code and give no guarantees as to if my
ideas will solve the problem. I also found the code did generated no
problem under BCB6 (XP Pro, 512M, PIII Mobile).
I assume that by this:
If not then the test without threads didn't proove much since the code which
the OS complains of does not execute.
If it were my code, I try a couple of things.
Increase the stack size to something like 16384, comment out creation of all
but 2 of the 9 image lists (list2 through list7, list8 is already commented
out) and put in code to check the return values from _beginthreadNT and
SetThreadPriority.
If I were getting very frustrated I'd probably also code to detect if
WaitForSingleObject(mThread, INFINITE); returned WAIT_ABANDONED.
Obviously some of the changes I mentioned may be difficult for the
production code. My goal in suggesting this is to find the problem or at
least generate a symptom related to a change so that the problem source can
be identified.
The RTL has undergone many changes in the 6 years since BC++ 5.02 was new.
If it still does not yield to debugging then I suggest you try one of these
two things. (Note that I've not used BC++ 5.02 for anything multi-threaded
in years, so consider the source.)
Set up a main thread (main program thread) delete function.
Call that routine with the address of the int** array from the class.
Let that function do the array deletion of the int** array and the int*
arrays it contains instead of deleting in the destructor.
Somehow arrange for a slight delay between when the thread ends and the
deletion.
or
Create a class which is essentially a container for the int** array and the
int* arrays it contains. That class should do the allocation and deletion.
Use an auto_pointer of that class as a member variable in the TVMImageList
class.
Or you might try both (but of course not together).
I am curious. Why do you do this:
for (int j=0; j < mLength ; j++){
array = thearray[j];
delete[] array;
}
Instead of this:
for (int j=0; j < mLength ; j++)
delete[] thearray[j];
. Ed

{smallsort}

Hi Ed,
Thanks for all your suggestions. I will go through each of them.
I just removed the thread creation and deletion, the array were still
allocated and deallocated.
preceding it to see what the compiler thought the size of the deallocation
should be.
I also tried testing the return codes for the 3 functions you mentioned but
they were all fine. Increasing the stack size did not help either.
I delayed the thread termination and the array deletion using Sleep and by
switching on the CodeGuard but still the same problem.
I added a delete function to the class to delete the arrays outside the
destructor. By calling the delete function for each instance before deleting
any of the class instances still caused the program to crash.
As an alternative we will look at using Builder 6 to generate the production
code as a DLL which we will link into our Borland C++ application. However,
the change in the name mangling of C++ functions means that we will have to
develop a C interface which is a bit of a pain. If you have any other
suggestion I would still be glad to hear them.
Thanks for the help
Martin
"Ed Mulroy [TeamB]" < XXXX@XXXXX.COM >wrote in message

obvious that you embraced it.
Do the whole thing in C++ Builder 6. Borland C++ is from 1997. At some
point the program is going to have to migrate anyway. Why not now?
. Ed

Martin Catney wrote:
order of your member variables, and destruct in reverse order of
construct. I don't think there should ever be a problem with reverse
order destruction.
 

We cannot move to Builder 6 as alot of our code is based on the OWL
libraries and re-coding the whole application will take some time, although
ultimately we will need to move to another library which may be VCL or CLX.
We have been using Builder 4 for years for all new dialogs but we cannot
move them to Builder 6 without some restructuring/recoding because of the
difference in the C++ name mangling between Builder 6 and Borland
5.02/Builder4.
So it looks like we are going to have to use Borland C++5.02, Builder 4 and
now Builder 6. A bit of a mess in the short term until we restructure the
code.
Martin
"Ed Mulroy [TeamB]" < XXXX@XXXXX.COM >wrote in message

Maybe I was not clear enough. With the actual data class I will be able to
delete the array in the reverse order but there will be a lot of other
allocations occurring with in the application which will be longer lived.
This causes the memory to be fragmented and at some point the deallocation,
or thread termination, is corrupting the memory or stack.
Martin
"Bob Gonder" < XXXX@XXXXX.COM >wrote in message

Martin Catney wrote:
the longer lived stuff sooner rather than later so that they _can_ be
destroyed in the proper order. ( or _not_ destroyed at all, but simply
reused....buffers or arrays mostly)
That is, you don't alloc the short-term stuff, then the long then
destroy the short.
 

So, even though you used BCB4 and it came with OWL you never moved to using
OWL in Builder? I think you have painted yourself into a corner. That
situation is not going to go away. It might be time to deal with it.
. Ed

We did not use Builder 4 with OWL for several reasons but I can not remember
why. Not having CodeGuard was one and I think we had probelms with the
16-bit VBX control we were using.
Do you know if any one has successfully compile OWL, or OWLNext, with
Builder 6?
Martin
"Ed Mulroy [TeamB]" < XXXX@XXXXX.COM >wrote in message

Hi, Martin
"Martin Catney" < XXXX@XXXXX.COM >wrote in message
6 and OWLNext 6.12. Its not painless, but it's a helluva lot less work than
re-writing the UI in Builder's VCL. Actually I am in the process of adding
some VCL forms to it as well. So far so good...
HTH,
Bruce
 

I remember messages from several people who said they were using OwlNext
with BCB6. I see that one other person has already posted a reply saying he
is working on doing it also.
. Ed