Unexplained Issue with Vector

This is the forum for miscellaneous technical/programming questions.

Moderator: 2ffat

Unexplained Issue with Vector

Postby theLizard » Wed Feb 08, 2017 5:48 pm

First up, I have been off line for a few months due to my wife having pancreatic cancer so have not as yet solved my problem re BUG in TIdTcpClient http://bcbj.org/forums/viewtopic.php?f=10&t=3061&sid=c659c44f1b9e1c34f7c921c49bc94a02 and TIdTcpClient - Memory Leak http://bcbj.org/forums/viewtopic.php?f=10&t=3059&sid=c659c44f1b9e1c34f7c921c49bc94a02

Getting back to my issues in attempting to rule in/out driver problems I have upgraded my system and reinstalled Berlin and all my custom controls onto the new system along with the project that is giving me the problem however I now find that what worked on my old system is not working on the new for unknown reason so am asking for any insight that would solve this problem.

The issue is to do with a vector std::vector<TLFrmClass *> forms;

In my components constructor i reserve x number of elements forms.reserve(20);

In a method i perform - std::vector<TLFrmClass*>::iterator found = std::find_if(forms.begin(), forms.end(), IsTitle(title));

Code: Select all
TForm* __fastcall TLFormsAgent::LoadForm(TForm* f, UnicodeString title, bool setCaption)
   {
   std::vector<TLFrmClass*>::iterator found = std::find_if(forms.begin(), forms.end(), IsTitle(title));

   if(found != forms.end())
      {
      (*found)->Form->BringToFront();
      return((*found)->Form);
      }
   else
      {
      AddForm(f, title, setCaption);
    found = std::find_if(forms.begin(), forms.end(), IsTitle(title));
      }
   if(found != forms.end())
      return((*found)->Form);
   else
      return(NULL);
   }



On this line of code I get an Access Violation, I place the forms in the Watch List and check it's values, the values reported are forms {???.???.???} but checking the same values in the constructor at start up after reserving x number of elements, they are forms {NULL,NULL,NULL} which to me seem to be correct.

Related to this line of code
Code: Select all
struct IsTitle
   {
   UnicodeString _title;

   IsTitle(UnicodeString title) : _title(title) {}

   bool operator()(TLFrmClass *cls) const
      {
      return (cls->Title == _title);
      }
   };


As stated both blocks of code work perfectly on the old system and have done so for a number of years without issue but now on the new system it breaks here without an explanation so am hoping that someone may point me in the right direction.

All project properties are set exactly the same on old and new system.

The old system is running Windows 10 Home, the new System is running Windows 10 Pro

TIA
theLizard
BCBJ Master
BCBJ Master
 
Posts: 439
Joined: Wed Mar 18, 2009 2:14 pm

Re: Unexplained Issue with Vector

Postby rlebeau » Thu Feb 09, 2017 11:44 am

theLizard wrote:In my components constructor i reserve x number of elements forms.reserve(20);


That simply allocates storage memory for 20 elements, but does not populate the vector with actual elements. Are you actually adding TLFrmClass objects to the vector at some point?

theLizard wrote:In a method i perform - std::vector<TLFrmClass*>::iterator found = std::find_if(forms.begin(), forms.end(), IsTitle(title));


That code can be simplified a little:

Code: Select all
TForm* __fastcall TLFormsAgent::LoadForm(TForm* f, UnicodeString title, bool setCaption)
   {
   std::vector<TLFrmClass*>::iterator found = std::find_if(forms.begin(), forms.end(), IsTitle(title));

   if (found == forms.end())
      {
      // I would suggest making AddForm() return the new TForm
      // that was added so you don't have to search for it...
      //
      // return AddForm(f, title, setCaption);
      AddForm(f, title, setCaption);
      found = std::find_if(forms.begin(), forms.end(), IsTitle(title));
      }

   if (found != forms.end())
      {
      (*found)->Form->BringToFront();
      return (*found)->Form;
      }

   return NULL;
   }


Alternatively:

Code: Select all
TForm* __fastcall TLFormsAgent::LoadForm(TForm* f, UnicodeString title, bool setCaption)
   {
   std::vector<TLFrmClass*>::iterator found = std::find_if(forms.begin(), forms.end(), IsTitle(title));
   if (found != forms.end())
      {
      (*found)->Form->BringToFront();
      return (*found)->Form;
      }

   // I would suggest making AddForm() return the new TForm
   // that was added so you don't have to search for it...
   //
   //return AddForm(f, title, setCaption);
   AddForm(f, title, setCaption);
   found = std::find_if(forms.begin(), forms.end(), IsTitle(title));
   if (found != forms.end())
      return (*found)->Form;

   return NULL;
   }


theLizard wrote:On this line of code I get an Access Violation


On *which* line of code exactly? You did not specify that.

theLizard wrote:I place the forms in the Watch List and check it's values, the values reported are forms {???.???.???}


That is usually an indication that you are inspecting invalid pointers. Possibly even the vector itself is being accessed via an invalid component pointer. Which might explain your crash.

theLizard wrote:but checking the same values in the constructor at start up after reserving x number of elements, they are forms {NULL,NULL,NULL} which to me seem to be correct.


That would make sense for a newly-constructed vector, and since you are debugging the component constructor, the component object pointer is still valid at that time.

theLizard wrote:Related to this line of code

Code: Select all
struct IsTitle
   {
   UnicodeString _title;

   IsTitle(UnicodeString title) : _title(title) {}

   bool operator()(TLFrmClass *cls) const
      {
      return (cls->Title == _title);
      }
   };



The only way that code can crash is if either:

1. you are storing invalid TLFrmClass* pointers in your vector. That has nothing to do with reserve(), only pointers that are actually pushed in are included in any iterations. What does AddForm() look like? What other code, if any, are you calling before calling LoadForm()? Either AddForm() is not allocating/pushing a TLFrmClass object correctly, or something else in your code is destroying the objects without removing them from the vector.

2. you are calling LoadForm() on an invalid TLFormsAgent pointer, thus std::find_if() is called on an invalid vector object, which might then call IsTitle::operator() with invalid TLFrmClass* pointers.

You need to do some further debugging. I would start and making sure the 'this' pointer inside of LoadForm() is correct to begin with.

theLizard wrote:As stated both blocks of code work perfectly on the old system and have done so for a number of years without issue but now on the new system it breaks here without an explanation so am hoping that someone may point me in the right direction.


There is not enough information provided to diagnose this one way or the other.

theLizard wrote:The old system is running Windows 10 Home, the new System is running Windows 10 Pro


The OS is not a factor in this issue.
Remy Lebeau (TeamB)
Lebeau Software
User avatar
rlebeau
BCBJ Author
BCBJ Author
 
Posts: 1375
Joined: Wed Jun 01, 2005 3:21 am
Location: California, USA

Re: Unexplained Issue with Vector

Postby theLizard » Thu Feb 09, 2017 2:47 pm

rlebeau wrote:That simply allocates storage memory for 20 elements, but does not populate the vector with actual elements. Are you actually adding TLFrmClass objects to the vector at some point?

Agreed.

rlebeau wrote:That code can be simplified a little:


Yes, I am aware that re-searching for the form is overkill but in this instance I believe it is necessary for the result I am after.

rlebeau wrote:On *which* line of code exactly? You did not specify that.


The crash occurs at 1==> in code below, stepping into IsTitle() before Access Violation is raised the crash is at 3==> in IsTitle() code block bellow.


Code: Select all
TForm* __fastcall TLFormsAgent::LoadForm(TForm* f, UnicodeString title, bool setCaption)
   {
1===>   std::vector<TLFrmClass*>::iterator found = std::find_if(forms.begin(), forms.end(), IsTitle(title));

   2==> if(found != forms.end())
      {
      (*found)->Form->BringToFront();
      return((*found)->Form);
      }
   else
      {
      AddForm(f, title, setCaption);
    found = std::find_if(forms.begin(), forms.end(), IsTitle(title));
      }
   if(found != forms.end())
      return((*found)->Form);
   else
      return(NULL);
   }


Code: Select all

struct IsTitle
   {
   UnicodeString _title;

   3==> IsTitle(UnicodeString title) : _title(title) {}

   bool operator()(TLFrmClass *cls) const
      {
      return (cls->Title == _title);
      }
   };

rlebeau wrote:That is usually an indication that you are inspecting invalid pointers. Possibly even the vector itself is being accessed via an invalid component pointer. Which might explain your crash.


Agreed, repeating the test just now the watch value for forms, at line 1==> i get { :00522644, NULL, NULL } where previous I got {???.???.???} but Access Violation is raised at 3==> in IsTitle()! so some inconsistencies going on (I think)

rlebeau wrote:That would make sense for a newly-constructed vector, and since you are debugging the component constructor, the component object pointer is still valid at that time.


Agreed, but nothing in any code alters the state after constructor initialization so, before any forms are added to the vector I should see (I think) {NULL, NULL, NULL}

rlebeau wrote:The only way that code can crash is if either:

1. you are storing invalid TLFrmClass* pointers in your vector. That has nothing to do with reserve(), only pointers that are actually pushed in are included in any iterations. What does AddForm() look like? What other code, if any, are you calling before calling LoadForm()? Either AddForm() is not allocating/pushing a TLFrmClass object correctly, or something else in your code is destroying the objects without removing them from the vector.

2. you are calling LoadForm() on an invalid TLFormsAgent pointer, thus std::find_if() is called on an invalid vector object, which might then call IsTitle::operator() with invalid TLFrmClass* pointers.


Code: Select all

TForm* __fastcall TLFormsAgent::AddForm(TForm* f, UnicodeString title, bool setCaption)
   {
   //insert the form f into forms array
   bool hasAgent = false;
   TLAgent *agent;
   TComponent *ctrl;

   int nextIndex = (int)forms.size();

   //does the form have a TLAgent
   for(int i = 0; i < f->ComponentCount; i++)
      {
      ctrl = f->Components[i];
      //if the form has an agent, set the FormsAgent property to this
      if((agent = dynamic_cast<TLAgent *>(ctrl)) != NULL)
         {
         hasAgent = true;
         agent->CaptionIsTitle = setCaption;
         agent->FormsAgent = this;
         break;
         }
      }

   TLFrmClass *newForm = new TLFrmClass();
   newForm->Form = f;
   if(hasAgent)
      {
      newForm->Agent = agent;
      agent->Title = title;
      newForm->Title = agent->Title;
      newForm->Agent->FormCreated = true;
      if(setCaption)
         newForm->Form->Caption = title;
      }

   newForm->IsSelected = true;
   newForm->indexOf = nextIndex;
   newForm->Title = title;

   forms.push_back(newForm);
   ActiveAgent = agent;

   if(FformsAgentFormAdded)
      FformsAgentFormAdded(this, newForm->Form, forms.size()-1);

   return(newForm->Form);
   }
//---------------------------------------------------------------------------



EDIT:

No other code is called prior to LoadForm(...) in fact, it crashes on the first call to LoadFor( .. ) which then calls AddForm( ... ) if an existing form is not found.

This is the code in form that calls LoadForm( ...) from main MDI form

Code: Select all

//form to load from TTreeView selected node

void __fastcall TfrmTv::tvDblClick(TObject *Sender)
   {
   switch(tv->Selected->Level)
      {
      case 1:

      frmMain->LoadForm(frmvehicledetails, "BimRMan: " + tv->Selected->Text);
         break;
    }
   }




Code: Select all

//get forms agent to create or bring to front requested form

bool __fastcall TfrmMain::LoadForm(int typeof, UnicodeString title)
   {

   int index = -1;
   switch(typeof)
      {
      case frmvehicledetails:
         if(!fVehicleDetails)
            {
            formsAgent->LoadForm(new TfrmVehicleDetails(this), title, true);
            }
         else
            fVehicleDetails->BringToFront();
         break;
      case frmpartsinv:
         if(!fPartsInvoice)
            {
            formsAgent->LoadForm(new TfrmPartsInvoice(this), title, true);
            }
         break;
      }
   }


rlebeau wrote:There is not enough information provided to diagnose this one way or the other.


Hope I have given a little bit more info to get to the bottom of the issue.

Thanks Remy for your help.
theLizard
BCBJ Master
BCBJ Master
 
Posts: 439
Joined: Wed Mar 18, 2009 2:14 pm

Re: Unexplained Issue with Vector

Postby rlebeau » Thu Feb 09, 2017 6:28 pm

theLizard wrote:The crash occurs at 1==> in code below, stepping into IsTitle() before Access Violation is raised the crash is at 3==> in IsTitle() code block bellow.


I can see a crash happening on line 1==> IF AND ONLY IF either:

1. the vector itself is being accessed by an invalid 'this' pointer.

2. IsTitle::operator() is crashing when accessing an invalid TLFrmClass pointer.

But the only possible way that line 3==> can be crashing is if your source UnicodeString is corrupted to begin with. All of the code related to line 3==> is just operating on local variables and copies of variables, so there is nothing to crash on under normal conditions.

theLizard wrote:
Code: Select all
if(!fVehicleDetails)
   {
   formsAgent->LoadForm(new TfrmVehicleDetails(this), title, true);
   }



Does the act of loading the form set the fVehicleDetails variable somewhere? Does the TfrmVehicleDetails object ever get destroyed, and if so does it get removed from the vector?

theLizard wrote:
Code: Select all
if(!fPartsInvoice)
   {
   formsAgent->LoadForm(new TfrmPartsInvoice(this), title, true);
   }



Same thing.

theLizard wrote:Hope I have given a little bit more info to get to the bottom of the issue.


Not really. You are just confusing the issue more.

I suggest you debug the code better. Take note of object pointer values and make sure those values remain the same at every step. Put breakpoints in destructors to make sure objects are not getting destructed unexpectedly. Use data breakpoints to detect if/when variables are getting modified unexpectedly. Etc Etc Etc.
Remy Lebeau (TeamB)
Lebeau Software
User avatar
rlebeau
BCBJ Author
BCBJ Author
 
Posts: 1375
Joined: Wed Jun 01, 2005 3:21 am
Location: California, USA

Re: Unexplained Issue with Vector

Postby theLizard » Fri Feb 10, 2017 2:06 pm

Thanks Remy,

rlebeau wrote:1. the vector itself is being accessed by an invalid 'this' pointer.


I cannot see how this would be possible.

rlebeau wrote:2. IsTitle::operator() is crashing when accessing an invalid TLFrmClass pointer.


This I can understand but at the time of crash there are no TLfrmClass pointers in the vector, the vector should only have the memory reserved for the first push of a TLfrmClass object which is created on the basis of there not being a TLfrmClass matching the criteria or the vector is empty as is the case.

rlebeau wrote:But the only possible way that line 3==> can be crashing is if your source UnicodeString is corrupted to begin with. All of the code related to line 3==> is just operating on local variables and copies of variables, so there is nothing to crash on under normal conditions.:


Agreed, I see now reason why it would do that here.

rlebeau wrote:Does the act of loading the form set the fVehicleDetails variable somewhere?


Yes, here in the FormsAgent FormAdded Event handler

Code: Select all
// set form temp variable according to ClassName()
void __fastcall TfrmMain::formsAgentFormAdded(TObject *Sender, TForm *form, int index)
   {

   if(form->ClassName() == "TfrmVehicleDetails")
      {
    frmVehicleDetails = static_cast<TfrmVehicleDetails*>(form);
    }
   }


rlebeau wrote:Does the TfrmVehicleDetails object ever get destroyed, and if so does it get removed from the vector?


Yes,under normal circumstances it is automatically destroyed and any associated variable deleted or set to NULL on form close but, at this point in trying to determine why I am getting a crash, the vector remains empty and the fVehicleDetails is not even assigned as the event is not triggered.


rlebeau wrote:I suggest you debug the code better. Take note of object pointer values and make sure those values remain the same at every step. Put breakpoints in destructors to make sure objects are not getting destructed unexpectedly. Use data breakpoints to detect if/when variables are getting modified unexpectedly. Etc Etc Etc.


I have pretty much already done that except in destructors because the crash occurs long before anything is destroyed.

I am going to try creating a new project from scratch with just a FormsAgent, an Agent and an empty child form and rule in or out any code I may have anywhere that may be causing the problem
theLizard
BCBJ Master
BCBJ Master
 
Posts: 439
Joined: Wed Mar 18, 2009 2:14 pm

Re: Unexplained Issue with Vector

Postby rlebeau » Sun Feb 12, 2017 2:09 am

theLizard wrote:
rlebeau wrote:1. the vector itself is being accessed by an invalid 'this' pointer.


I cannot see how this would be possible.


It is the only thing that makes sense to explain your symptoms.

theLizard wrote:This I can understand but at the time of crash there are no TLfrmClass pointers in the vector


Then either you are accessing an invalid vector object, or the UnicodeString is corrupt. Those are the only things the code can crash on. I'm suspecting an invalid object pointer. I suggest you examine the 'this' pointer inside the constructor that reserves the vector memory, and then examine the 'this' pointer inside of LoadForm(), and make sure they are the same pointer value.

theLizard wrote:
Code: Select all
if(form->ClassName() == "TfrmVehicleDetails")
    {
    frmVehicleDetails = static_cast<TfrmVehicleDetails*>(form);
    }



You should be using dynamic_cast instead:

Code: Select all
if (TfrmVehicleDetails *details = dynamic_cast<TfrmVehicleDetails*>(form))
    {
    frmVehicleDetails = details;
    }
Remy Lebeau (TeamB)
Lebeau Software
User avatar
rlebeau
BCBJ Author
BCBJ Author
 
Posts: 1375
Joined: Wed Jun 01, 2005 3:21 am
Location: California, USA

Re: Unexplained Issue with Vector

Postby theLizard » Sun Feb 19, 2017 4:29 pm

rlebeau wrote:I am going to try creating a new project from scratch with just a FormsAgent


As suspected, on creating a new project and placing an TFormsAgent on the MDI parent then creating a new child form using TButton's OnClick event ( fagent->LoadForm(new Tfrm1(this), "Test Form", true); ) it works without issue so, there is a problem somewhere in the project creation .

theLizard wrote:You should be using dynamic_cast instead:


TBH, I did not think of doing that but makes sense as I use that technique all the time in other places, other things on my mind I guess.

Thanks Remy,

Chyeers
theLizard
BCBJ Master
BCBJ Master
 
Posts: 439
Joined: Wed Mar 18, 2009 2:14 pm


Return to Technical

Who is online

Users browsing this forum: Yahoo [Bot] and 4 guests