Array access violation on program shutdown 1

[YishGene]

Hi,

I have a problem with a large program I wrote, under certain conditions I get access violations when I exit the program (but not as far I tried when it's still running).

I've narrowed it down to the following part, it's a routine that only gets called by the object's constructor (FX- and FYmotionTables are private members of TElementImageTranslation):

procedure TElementImageTranslation.GenerateXYmotionTables;
var
   NFrames, FirstFrame: Integer; //number of frames for motion
   MovePerSec, MovePerFrame, PixPerDeg: Double; //number of pixels to move per second/frame
   i, FrameRate: Integer;
   MovingTable, StillTable: ArrSmallInt;
   isNegative: Boolean;
begin
   FrameRate := vsgGetSystemAttribute(vsgFRAMERATE);
   NFrames := Max(Ceil((Offset - Onset)*FrameRate),1);

...

   //allocate memory for the XYmotion tables
   SetLength(FXmotiontable, NFrames);
   SetLength(FYmotiontable, NFrames);

   //translate the deg/sec units to pix/sec

   vsgUnitToUnit(vsgDEGREEUNIT,1,vsgPIXELUNIT,PixPerDeg);
   MovePerSec := SpeedTranslation * PixPerDeg;

    // Video pages go in the Y direction and depend on how many frames in this direction
    // fit on the video page (which is 1024 pixels high)
    NVideoPages := ceil((NFrames*MovePerSec/FrameRate+480)/1024);

   FirstFrame := round(FStartingAngle*PixPerDeg);

   [b]case FTransDirection of
   TTD_Left: begin
      MovePerFrame :=MovePerSec/FrameRate;
      MovingTable := FXmotiontable;
      StillTable := FYmotiontable;
   end;
   TTD_Right: begin
      MovePerFrame := -MovePerSec/FrameRate;
      MovingTable := FXmotiontable;
      StillTable := FYmotiontable;
   end;
   TTD_Up: begin
      MovePerFrame := -MovePerSec/FrameRate;
      MovingTable := FYmotiontable;
      StillTable := FXmotiontable;
   end;
   TTD_Down: begin
      MovePerFrame := MovePerSec/FrameRate;
      MovingTable := FYmotiontable;
      StillTable := FXmotiontable;
   end;
   end;
[/b]

       isNegative := false;
      for i:= 0 to NFrames do
      begin
         StillTable[i] := 0;
         MovingTable[i] :=  FirstFrame + round(i*MovePerFrame);
         isNegative := isNegative or (MovingTable[i] < 0);
      end;

...

end;

If I comment the part in bold, and replace the last loop with:

for i:= 0 to NFrames do
      begin
         FXmotiontable[i] := 0;
         FYmotiontable[i] :=  FirstFrame + round(i*MovePerFrame);
         isNegative := isNegative or (FYmotiontable[i] < 0);
      end;

Then I get no crashes at program shutdown anymore (although the functionality of the code is lost, because it only addresses one case. Strangely enough, these crashes only happen if during runtime I generate 3 or more of these objects (each separately by allocating a pointed and calling the constructor).

I realize I can get around the issue in various ways, but my question here is about understanding the reference count that is supposed to control allocation of memory for the arrays. As I understand it, after the following lines:

TTD_Down: begin
      MovePerFrame := MovePerSec/FrameRate;
      MovingTable := FYmotiontable;
      StillTable := FXmotiontable;
   end;

We have MovingTable and StillTable referencing the same arrays as FYmotiontable and FXmotiontable respectively, and this bears out because the assignments in the loop correctly change the values in the two latter arrays. When the function finishes, MovingTable and StillTable die, bumping the references to FYmotiontable and FXmotiontable down to one each, so when these objects are destroyed, there are only these references left and the garbage collector should free the memory as expected. But it doesn't, and I get the access violations when the program terminates (but not when I dereference the objects themselves).

What am I getting wrong here? Any suggestions?

 

[svanels]

Correct me if I am wrong, but is this plain turbo pascal to be used in dos crt mode? I cannot detect any windows components.

Steven

 

[YishGene]

svanels,

Sorry for being unclear, this is Delphi 6. The Windows components are there but irrelevant to this problem afaik. The entire code is far to big to post here so I cut out the part I believe contains the problem.

Thanks !

 

[whosrdaddy]

copy the table instead of referencing it:

TTD_Down: begin
      MovePerFrame := MovePerSec/FrameRate;
      MovingTable := Copy(FYmotiontable);
      StillTable := Copy(FXmotiontable);
   end;

-----------------------------------------------------
What You See Is What You Get
Never underestimate tha powah of tha google!

 

[YishGene]

From Copy's help file:

Note: When S is a dynamic array, Copy can only be used as a parameter in a call to a procedure or function that expects an array parameter. That is, it acts like the Slice function when working with dynamic arrays.

Besides, part of the point of this code is that the assignments in the last loop affect FY- and FXmotiontable, not a local copy of them that subsequently dies when the function finishes.

 

[whosrdaddy]

Ok, should have seen that

A dynamic array has the same reference counting as a longstring, so the code should pose no problem.
if you want to dereference directly set

MovingTable and StillTable to nil at the end of the method

the worrying part is this :

Strangely enough, these crashes only happen if during runtime I generate 3 or more of these objects (each separately by allocating a pointer and calling the constructor).

can I see (part of) the code where you create theTElementImageTranslation objects?
/Daddy


-----------------------------------------------------
What You See Is What You Get
Never underestimate tha powah of tha google!

 

[YishGene]

OK thanks for your help whosrdaddy.
Adding

   MovingTable := nil;
   StillTable := nil;

at the end of the function does not fix the crashes unfortunately.

Here's one example of code creation from a file, which also makes clear that I'm a complete novice... Pointers are simple and straightforward in C and C++, but I just can't seem to grasp them correctly in Delphi:

procedure TStimuliINIFile.ReadElement(ElementNode: TTreeNode; Section: String);
var
   element_type, spacial_wave_type, contrast_mod_freq_wave,readstr: String;
   pNewElement: PElement;
   TransDirection: TETransDirection;
begin
   element_type := ReadString(Section,'ElementType','');
   New(pNewElement);

   ...
   //Checks different types of Elements here... very sloppy code I admit because I was learning Delphi as I wrote through this project... 
   
      else if element_type = 'ImageTranslation' then
      begin
         readstr := ReadString(Section, 'TransDirection','Left');
         if readstr = 'Left' then
            TransDirection := TTD_Left
         else if readstr = 'Right' then
            TransDirection := TTD_Right
         else if readstr = 'Up' then
            TransDirection := TTD_Up
         else //readstring = 'Down'
            TransDirection := TTD_Down;
         pNewElement^ := TElementImageTranslation.Create(
                           TransDirection,
                           ReadFloat(Section, 'SpeedTranslation', 0),
                           ReadFloat(Section, 'StartingAngle', 0),
                           ReadString(Section, 'BitmapFileName', ''),
                           ReadFloat(Section, 'Onset',0),
                           ReadFloat(Section, 'Offset',0));
      end
      else ...

   // For all Element types: store created element in treeview.  
   ElementNode.Data := pNewElement;
   ElementNode.Text := PElement(ElementNode.Data)^.Name;
end;

The constructor interacts with hardware most of the time and then calls GenerateXYmotionTables (the function in my first post) once.

The TElementNode objects get stored in a descendant of a TTreeView that sits in the main form.

And yes, I'm also completely flummoxed with this 3 object thing, I have no idea how to explain it and why is 3 the threshold rather than 2.

Thanks again for looking into this, if you need more code snippets I'll gladly supply.

 

[whosrdaddy]

get rid of the PElement thing, it is not needed!

I suppose you have other classes like TElementImageTranslation?

what is the definition of PElement?





-----------------------------------------------------
What You See Is What You Get
Never underestimate tha powah of tha google!

 

[YishGene]

PElement is simply

type PElement = ^TElement;

, I need the pointer type to allocate memory for the TElement objects so I can store them in the TTreeNodes in their Data properties. Or so I think at least, I remember spending many hours until I got this to work without access violations.

Yes, there are other classes such as TElementBackground, TElementRectangularObject, TElementRoundObject, TElementImageRotation, TElementWindmill etc. none of them cause any similar problem.

 

[whosrdaddy]

and those classes inherit from TElement?

then why bothering with the pointer? functions like New and Dispose are really oldschool and not really in line with oop

procedure TStimuliINIFile.ReadElement(ElementNode: TTreeNode; Section: String);
var
   element_type, spacial_wave_type, contrast_mod_freq_wave,readstr: String;
   NewElement: TElement;
   TransDirection: TETransDirection;
begin
   element_type := ReadString(Section,'ElementType','');
   ...
   //Checks different types of Elements here... very sloppy code I admit because I was learning Delphi as I wrote through this project... 
   
      else if element_type = 'ImageTranslation' then
      begin
         readstr := ReadString(Section, 'TransDirection','Left');
         if readstr = 'Left' then
            TransDirection := TTD_Left
         else if readstr = 'Right' then
            TransDirection := TTD_Right
         else if readstr = 'Up' then
            TransDirection := TTD_Up
         else //readstring = 'Down'
            TransDirection := TTD_Down;
         NewElement := TElementImageTranslation.Create(
                           TransDirection,
                           ReadFloat(Section, 'SpeedTranslation', 0),
                           ReadFloat(Section, 'StartingAngle', 0),
                           ReadString(Section, 'BitmapFileName', ''),
                           ReadFloat(Section, 'Onset',0),
                           ReadFloat(Section, 'Offset',0));
      end
      else ...

   // For all Element types: store created element in treeview.  
   ElementNode.Data := NewElement;
   ElementNode.Text := TElement(ElementNode.Data).Name;
end;

modified and should work.

One question though, are you freeing the created elements somewhere in code? (if not, you have some nice memoryleaks)


-----------------------------------------------------
What You See Is What You Get
Never underestimate tha powah of tha google!

 

[YishGene]

OK thanks a lot, I'll try this next time I'm at work (need the hardware to run it).

One thing I don't understand from your post is - why in your code do I need to free the created objects? I thought this should be dealt with by the garbage collector.

Do I need to .Free them or how should I go about this?

And if I'm already at it, you mention these methods are oldschool, but I find them in every textbook or online site I look. Do you know of any such resources that skip the mummytech and gets you straight to the modern things?

 

[YishGene]

OK forget my (stupid) remark about the garbage collector, I see your point and will implement it.

 

[whosrdaddy]

this is not .NET, you are responsible for freeing the objects you created.

if you don't want to keep track of the objects you created, use a TObjectList and create it with AOwnsObjects parameter (True). this means that when the object is deleted from the list, it is automatically freed. You can find the TObjectList in the Contnrs unit.

Cheers,
Daddy

-----------------------------------------------------
What You See Is What You Get
Never underestimate tha powah of tha google!