Crazy Access Violation in C++ Builder 6

I have the following code, and for the life of me, I cannot understand why there would be an Access Violation exception? I even deleted all the OBJs, TDS etc files and put it into a new project, still the Access Violation occurs.

Essentially, this code displays a TListView in a TFrame and is to show the various current times around the world for different time zones.

Note: The code is in C++ Builder 6.

Can someone help?

BLOODY-HELL-UPDATE: Solved. I should not add items to TListView in the TFrame constructor. DUMB DUMB DUMB.

MAJOR UPDATE: It seems that when the UpdateTimes() is called via the timer, the "li->Deleting" property is TRUE. When called outside the timer, it is FALSE. Now why would "li->Deleting" be set to 'true' because it is called from the timer? If i do:

if(li->Deleting == false)
{
  li->Caption = "abcd";
}

It doesnt enter the if(), when UpdateTimes() is called from the timer...... argggggh!!!

UPDATE: It seems like if I call UpdateTimes() outside of the TTimer, it works fine. But when called from the timer, it throws the Access Violation. What gives?

Header File:

#ifndef CurrentTimes_FrameH
#define CurrentTimes_FrameH
#include <Classes.hpp>
#include <Controls.hpp>
#include <StdCtrls.hpp>
#include <Forms.hpp>
#include <ExtCtrls.hpp>
#include <ComCtrls.hpp>
#include <list>
using namespace std;
//---------------------------------------------------------------------------
struct LOCATIONTIMEINFORMATION
{
  AnsiString TimeZoneName;
  AnsiString PlaceName;
  int    UtcOffsetMinutes;
  TListItem* ListItem;
};
//---------------------------------------------------------------------------
class TCurrentTimesFrame : public TFrame
{
__published:    // IDE-managed Components
    TTimer *Timer;
    TListView *ListView;
    void __fastcall TimerTimer(TObject *Sender);
private:    // User declarations
public:     // User declarations
    __fastcall TCurrentTimesFrame(TComponent* Owner);
//---------------------------------------------------------------------------
//User Code
//---------------------------------------------------------------------------
private:
    list<LOCATIONTIMEINFORMATION>   FTimeInformation;
  typedef list<LOCATIONTIMEINFORMATION>::iterator LocationTimeInformationItr;
public:
  void AddTimeInformation(LOCATIONTIMEINFORMATION lti);
  void UpdateTimes();
};
//---------------------------------------------------------------------------
#endif

CPP File:

#include <vcl.h>
#pragma hdrstop
#include "CurrentTimes_Frame.h"
#pragma package(smart_init)
#pragma resource "*.dfm"
//---------------------------------------------------------------------------
__fastcall TCurrentTimesFrame::TCurrentTimesFrame(TComponent* Owner): TFrame(Owner)
{
  Timer->Enabled = false;
  <strike>{
    LOCATIONTIMEINFORMATION lti;
    lti.TimeZoneName = "UTC";
    lti.PlaceName = "Near Greenwich, England";
    lti.UtcOffsetMinutes = 0;
    AddTimeInformation(lti);
  }</strike>
  //UPADTED: Don't add TListItem from constructor 
}
//---------------------------------------------------------------------------
void TCurrentTimesFrame::AddTimeInformation(LOCATIONTIMEINFORMATION lti)
{
  TListItem* li = ListView->Items->Add();
  li->Caption = lti.TimeZoneName;
  li->SubItems->Add(lti.PlaceName);
  li->SubItems->Add(lti.UtcOffsetMinutes);
  li->SubItems->Add("<time will come here>");
  lti.ListItem = li;
  ShowMessage(AnsiString(lti.ListItem->ClassName())); //Correctly shows "TListItem"
  FTimeInformation.push_back(lti);

  {
  LOCATIONTIMEINFORMATION temp = FTimeInformation.front();
  ShowMessage(AnsiString(temp.ListItem->ClassName())); //Correctly shows "TListItem"
  }
  Timer->Enabled = true;
}
//---------------------------------------------------------------------------
void __fastcall TCurrentTimesFrame::TimerTimer(TObject *Sender)
{
    UpdateTimes();
}
//---------------------------------------------------------------------------
void TCurrentTimesFrame::UpdateTimes()
{
  Timer->Enabled = false;
  TListItem* li;
  for(LocationTimeInformationItr itr=FTimeInformation.begin();itr!=FTimeInformation.end();itr++)
  {
    li = itr->ListItem;

    ShowMessage(AnsiString(li->ClassName())); //Access Violation:
    /*
    ShowMessage() above shows:

    ---------------------------
    Debugger Exception Notification
    ---------------------------
    Project XX.exe raised exception class EAccessViolation with message 'Access violation at address 4000567D in module 'rtl60.bpl'. Read of address 00000000'. Process stopped. Use Step or Run to continue.
    ---------------------------
    OK   Help
    ---------------------------
    */
  }
  Timer->Enabled = true;
}
//---------------------------------------------------------------------------

UPDATE A sample code demo'ing that list takes items as copy, not reference. (As far as I can see, please correct me if im making some mistake in the code below)

@Craig Young:

I'm confused... I thought structs would be added to the list as a copy not as a reference? Please take a look at the code below, it seems that a copy is being made? Or am I missing something rudimentary? Or a coding mistake below??

void PopulateData()
{
    AnsiString DebugText;
    list<LOCATIONTIMEINFORMATION> Data;

  LOCATIONTIMEINFORMATION OnStack;

  //Prints "junk"
  DebugText.sprintf("%s,%s,%d,%d",OnStack.TimeZoneName,OnStack.PlaceName,OnStack.UtcOffsetMinutes,(int)OnStack.ListItem);

    OnStack.TimeZoneName = "UTC";
    OnStack.PlaceName = "Near Greenwich, England";
    OnStack.UtcOffsetMinutes = 10;
    OnStack.ListItem = (TListItem*)20;

  //OnStack:
  DebugText.sprintf("%s,%s,%d,%d",OnStack.TimeZoneName,OnStack.PlaceName,OnStack.UtcOffsetMinutes,(int)OnStack.ListItem);
  //Add data to list
    Data.push_back(OnStack);

  //Get struct from list
  LOCATIONTIMEINFORMATION InList = Data.front();

  //OnStack:
  DebugText.sprintf("%s,%s,%d,%d",OnStack.TimeZoneName,OnStack.PlaceName,OnStack.UtcOffsetMinutes,(int)OnStack.ListItem);
  //InList:
  DebugText.sprintf("%s,%s,%d,%d",InList.TimeZoneName,InList.PlaceName,InList.UtcOffsetMinutes,(int)InList.ListItem);

  //Change OnStack
    OnStack.TimeZoneName = "NONE";
    OnStack.PlaceName = "USA";
    OnStack.UtcOffsetMinutes = 50;
    OnStack.ListItem = (TListItem*)90;

  //OnStack:
  DebugText.sprintf("%s,%s,%d,%d",OnStack.TimeZoneName,OnStack.PlaceName,OnStack.UtcOffsetMinutes,(int)OnStack.ListItem);
  //InList:
  DebugText.sprintf("%s,%s,%d,%d",InList.TimeZoneName,InList.PlaceName,InList.UtcOffsetMinutes,(int)InList.ListItem);

  //Change InList:
    InList.TimeZoneName = "SOME";
    InList.PlaceName = "BRAZIL";
    InList.UtcOffsetMinutes = 66;
    InList.ListItem = (TListItem*)88;

  //OnS开发者_StackOverflow中文版tack:
  DebugText.sprintf("%s,%s,%d,%d",OnStack.TimeZoneName,OnStack.PlaceName,OnStack.UtcOffsetMinutes,(int)OnStack.ListItem);
  //InList:
  DebugText.sprintf("%s,%s,%d,%d",InList.TimeZoneName,InList.PlaceName,InList.UtcOffsetMinutes,(int)InList.ListItem);
}

EDIT: My answer is incorrect, I've decided to leave it in place because it is worthwhile bearing in mind that if your collection (list) holds elements by reference, this is a very real possibility for 'strange access violations'. The symptoms described would correlate perfectly had it not been for the STL list keeping a copy of the elements.

Hi Liao,

You wrote: "BLOODY-HELL-UPDATE: Solved. I should not add items to TListView in the TFrame constructor."

I'm going to disagree with you; you have not solved it. While not necessarily a good idea (in terms of design), adding items to TListView in the TFrame constructor should not cause access violations.

EDIT: Despite my answer below being incorrect, I still disagree with Liao's 'BLOODY-HELL-UPDATE'. Adding items to TListView in the TFrame constructor should not cause access violations. In fact I took the original code and tested it in CPBB 2009, and it worked perfectly. This suggests that the error may have been in how the frame was used; or some other aspect of the code that was not demonstrated.

The problem is with the following line in the constructor:

LOCATIONTIMEINFORMATION lti;

• This allocates lti on the stack.
• You then add lti to a list; or more correctly: you add a reference to lti to the list.
• When your constructor goes out of scope, so does lti; and that memory can be reused by any other part of your application.
• Later when your timer attempts the update, the reference in FTimeInformation is still there.
• You use this reference to lookup where lti was.
• If that section of memory has been changed by any other part of your application, then ltr->ListItem no longer references the TListItem that was created in the constructor. Instead it references some other part of memory that it tries to use as if it were a TListItem. Therefore you experience 'strange' problems such as:
  • li->Deleting == false
  • li->ClassName causing an access violation.

NOTE: Whether or not you actually get an access violation usually depends a bit on luck: Consider yourself lucky if you do get the access violation; the other option is usually 'inexplicable' erratic behaviour.

Try modifying your constructor as follows, it should fix the access violation. NOTE: lti is now dynamically allocated, you'll have to decide when to free it, otherwise you'll have a memory leak ;)

LOCATIONTIMEINFORMATION* lti = new LOCATIONTIMEINFORMATION;
lti->TimeZoneName = "UTC";
lti->PlaceName = "Near Greenwich, England";
lti->UtcOffsetMinutes = 0;
AddTimeInformation(*lti);

I can't actually see any problem with the code.

Try printing the TimeZoneName or PlaceName from your iterator, rather than li->ClassName(), to make sure you haven't accidentally added something else to the list or something...

What values do you have in FTimeInformation? For example, is li == NULL?

If we can assume that the access violation occurs the first time through the loop, and li points to a valid TListItem then perhaps we should split the three statements on the line over three lines. Something like this:

const char* className = li->ClassName();
AnsiString  ansiString(className);
ShowMessage(ansiString);

If the access violation doesn't happen on the first line this will tell us something interesting.