Crash when allocating a new object Topic is solved

[spamiam]

I have a class that dynamically allocated memory for 2 arrays. In the class destructor I have:

Code: Select all

delete [] array1;
delete [] array2;

In the instantiating class header I have a statement like:

Code: Select all

myClass *obj;

And then in the instantiating cpp file the object is instantiated like this

Code: Select all

obj = new myClass(wxString filename);

In that same method later I have the following code:

Code: Select all

delete obj;
obj = NULL;
obj = new myClass(wxString filename);

I get a crash when running without the debugger, and an error saying "segmentation error" when using the debugger.

Any ideas? It appears to me that the problem is in the destruction of the class causing memory problems, but I can't tell where or why (but then I am very new to C++)! I can post more complete code if it seems useful to anyone.

-Tony

[Auria]

Can you use a debugger to get a backtrace so we know where it crashes?

[spamiam]

Can you use a debugger to get a backtrace so we know where it crashes?

Thanks for helping. I am using wxDevC++. I think it has some sort of a backtrace ability. I will check it out and see what it has.

-Tony

[spamiam]

Here is the backtrace as listed by wxdevC++. Remember, it works the firsat time I open a data file and load the data, and crashes the second time I try it.

.
.
.
AnalyzerFrm::FileOpenClick
wxEvtHandler::ProcessEventMatches
wxEventHashTable::HandleEvent
wxEvtHandler::ProcessEvent
wxFrameBase::ProcessCommand
wxFrame::MSWWindowProc
wxWndProc
USER32!GetDC
non-Virtual thunk to wxListBoxBase::DoInsert(wxString const&, unsigned int)
??
??
??

And here is the part that deletes the old object and instantiated the new one. I presume that it is one of these operations that is crashing the system

Code: Select all

void AnalyzerFrm::FileOpenClick(wxCommandEvent& event)
{
    wxFileDialog dialog (this, _T("Open XXX File"), _T(""), _T(""),
                          _T("XXX Files (*.sla)|*.sla"), wxOPEN | wxFILE_MUST_EXIST | wxCHANGE_DIR);
    if(dialog.ShowModal() == wxID_OK)
    {
        wxString filename = "";
        filename = dialog.GetPath();
        //delete the old data set
        if (capturedData) 
        {
            delete capturedData;
            capturedData = NULL;
        }
        
        capturedData = new CapturedData(filename);
        if (capturedData)
        {
            wxString text;
            text = "XX XXXXX XXXXXXXX  " ;
            text.Append(filename);
            SetTitle(text);

            text.Printf(wxT("XXXXXXX XXXX: %dHz"),capturedData->GetRate());
            MenuBar->SetMenuLabel(RATETEXT,text);
            
            text.Printf(wxT("  XXXXXX: %d samples"),capturedData->GetLength());
            MenuBar->SetMenuLabel(LENGTHTEXT,text);
            
            text.Printf(wxT("  XXXXXXX: %7.3f sec"),(float)capturedData->GetLength()/(float)capturedData->GetRate() );
            MenuBar->SetMenuLabel(DURATIONTEXT,text);
        }

        
        //now try to load data from the file.
    }
    else
    {
        //TODO: Throw an exception
    }
    
}

Here is the destructor of my derived class:

Code: Select all

CapturedData::~CapturedData(void)
{
    delete [] timestamps;
    delete [] values;
}

And here is the class declaration from the header file

Code: Select all

class CapturedData// : public wxApp
{
  .
  .
  .    
    long *timestamps;
    int *values;
    
    
    
    //private methods
    int     CapturedData::ReadLine(wxFile *dataFile, wxString *line);
    void    CapturedData::ParseLine(wxString *line);
    void    CapturedData::InitVars(void);


	public:
        CapturedData(wxString filename);
        CapturedData::~CapturedData(void);
        .
        .
        .
};

[Auria]

Your backtrace seems to miss a lot of information, did you build your project with debug symbols (in debug mode) ?

Can we also see where you create timestamps and values?
A minimal compilable code sample demonstrating the issue is often the best way to get it fixed

[spamiam]

I thought that it was compiled in debug mode, but I can try again. I am working on the minimal version that replicates the problem right now.

-Tony

[spamiam]

Well, I got the problem solved. It was one factor I did not expect (aren't they all!).

the problem was in a method in the object that gets destroyed and re-instantiated. I knew that the crash had something to do with allocating the object again, or with deleting the old one.

It turns out that it was a problem with a 'static' variable. I had wanted to have a variable whose value was retained across calls to a method (one which parses a lane read from a file for the putpose of filling data arrays).

so I wrote the method thusly:

Code: Select all

void CapturedData::ParseLine(wxString *line)
{
    wxString variable;
    wxString value;
    size_t length;
    long nValue;
    static int ptr = 0;
    int loc;
    .
    .
    .
    loc = line->Find((wxChar)'@');
    value = line->Mid(0,loc);
    if (!value.ToLong(&nValue,16) )nValue = 0;
    values[ptr] = (int)nValue;
        
    value = line->Mid(loc+1);
    if (!value.ToLong(&nValue,10) )nValue = 0;
    timestamps[ptr] = nValue;
    
    ptr++;
    .
    .
    .

When I make the 'ptr' variable a private class variable, which is initialized to zero in the constructor, then everything works. When I simply make the scope of the 'ptr' variable just the method (and make it static in the method), then it does not work.

I had thought that it was OK to allocate static variables inside methods and it will work as I had expected!

[Auria]

I haven't checked the whole code, but my guess is you're not aware that this line :

Code: Select all

static int ptr = 0; 

will only be executed once. So this means that if you create many objects, or call the function many times, ptr will not reset to zero, and will keep its previous value, probably resulting in array-out-of-bounds overflows. static variables are really meant for things that never change no matter what is instanciated/deleted (hence the name 'static')

[spamiam]

Code: Select all

static int ptr = 0; 

will only be executed once. So this means that if you create many objects, or call the function many times, ptr will not reset to zero, and will keep its previous value, probably resulting in array-out-of-bounds

Well, that is exactly what was happening! But I thought (apparently incorrectly) that 'static' had two meanings depending on the scope. Class-scoped statics, I thought, acted the way you describe: there is one variable for the entire class. I thought that the method-scoped version had a unique static variable for each individual object.

I even went back to some on-line sources and re-read the definition of static, and they SEEMED to differentiate them based on their placement in a class or a method.

Even when I went back and put a watch on the 'ptr' variable, it was not instantly obvious that ptr was a problem. At first it seemed to get re-initialized to zero, but then it jumped back to where the last instance (now deleted) left off.

So, I am left with the necessity of declaring a class-scope private variable that only gets used inside one method in order to be able to retain the value of the variable across calls to the method. I much much prefer having variables that only apply to a certain method scoped only to that method.

Is there any way I can have a variable declared within a method retain its value across calls to the method while allowing each instance of the class to have its own unique version?

-Tony

[Auria]

I thought that the method-scoped version had a unique static variable for each individual object.

Not sure what you read; in all languages I've ever used, 'static' is actually is a way to circumvent per-instance variables - otherwise what would be the difference with member variables?

Is there any way I can have a variable declared within a method retain its value across calls to the method while allowing each instance of the class to have its own unique version?

I don't know the entire C++ specs, but I don't know anything elegant allowing this, sorry.
Maybe you could change the architecture, e.g. using a loop instead of repetitive calls.

[spamiam]

Not sure what you read; in all languages I've ever used, 'static' is actually is a way to circumvent per-instance variables - otherwise what would be the difference with member variables?

All I can say is that the sources I read seemed to suggest that class-scope statics were different from method-scope statics in some way other than just the scope. My mistake!

otherwise what would be the difference with member variables?

Well, the difference would just be _scope_. Following the paradigm of "encapsulation" one should expose variables only as widely as necessary. Therefore, there SHOULD be an opportunity for a method-scope, per-instance persistent variable, just as there is a class-scope per-instance type of variable (which automatically persists).

I got over my philosophical aversion to opening up the visibility of the variable(s) and everything works fine now. From now on I will try to forget the keyword "static" except for variables that I want to be set for ALL class members. I suppose it is my old C (not ++) training where I made lots of use of static variables inside individual functions, as well as inside modules.

Thanks to everyone for helping me understand this (to me) confusing issue.

-Tony

[upCASE]

Hi!
I didn't read everything, but maybe namespaces might help you.

[spamiam]

Hi!
I didn't read everything, but maybe namespaces might help you.

I will have to check that out. Being new to modern C++, I am not aware namespaces other than the fact that they exist.

-Tony

[Auria]

Hi!
I didn't read everything, but maybe namespaces might help you.

Namespaces are indeed great, but I fail to see how they can solve the OP's issue