View Issue Details

IDProjectCategoryView StatusLast Update
0023851LazarusLCLpublic2013-02-11 14:12
ReporterBart BroersmaAssigned ToBart Broersma 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformi386OSWindowOS VersionWin7
Product Version1.1 (SVN)Product Buildr40197 
Target VersionFixed in Version1.1 (SVN) 
Summary0023851: ValueListEditor crash on Strings.Delete
DescriptionValueListEditor crashes with AV when you delete more than one item from the Strings property

Tested with r40197
(At first I thought I introduced this myself with r40204, but this is not the case)
Steps To ReproduceValEd is a TValuelistEditor.

 if ValEd.Strings.Count > 0 then ValEd.Strings.Delete(0);

Run this code twice on a ValueListEditor

Build and run attached demo.
Pres button "Delete(0)" twice --> Crash
Additional InformationHere's the output on the console.

C:\Users\Bart\LazarusProjecten\bugs\ValEdit>vle
TApplication.HandleException Access violation
  Stack trace:
  $0056399F TVALUELISTEDITOR__SELECTVALUEEDITOR, line 600 of valedit.pas
  $00545E85 TCUSTOMGRID__SELECTEDITOR, line 7708 of grids.pas
  $0053961F TCUSTOMGRID__SETOPTIONS, line 2472 of grids.pas
  $00563C16 TVALUELISTEDITOR__SETOPTIONS, line 694 of valedit.pas
  $005632E7 TVALUELISTSTRINGS__DELETE, line 449 of valedit.pas
  $00427578 TFORM1__BUTTON3CLICK, line 74 of main.pp
  $004FC242 TCONTROL__CLICK, line 2718 of ./include/control.inc
  $0051367F TBUTTONCONTROL__CLICK, line 55 of ./include/buttoncontrol.inc
  $00513C6F TCUSTOMBUTTON__CLICK, line 175 of ./include/buttons.inc
  $005142C1 TBUTTON__CLICK, line 355 of ./include/buttons.inc
  $005135BA TBUTTONCONTROL__WMDEFAULTCLICKED, line 26 of ./include/buttoncontr
ol.inc
  $0040AE56
  $004F028E TWINCONTROL__WNDPROC, line 5323 of ./include/wincontrol.inc
  $00537B35 DELIVERMESSAGE, line 117 of lclmessageglue.pas
  $004D3063 WINDOWPROC, line 2476 of ./win32/win32callback.inc
  $00560488 CUSTOMFORMWNDPROC, line 375 of ./win32/win32wsforms.pp
  $750B62FA
TagsNo tags attached.
Fixed in Revisionr40209
LazTarget-
Widgetset
Attached Files

Activities

Bart Broersma

2013-02-07 20:25

developer  

ValEdit.zip (3,298 bytes)

Bart Broersma

2013-02-07 20:46

developer   ~0065553

The result of GetItemProp in TValueListEditor.SelectValueEditor after the second Delete() is DEADBEEF.

Jesus Reyes

2013-02-08 09:20

developer   ~0065571

seems the problem here is that FItemProps is not in sync with list items, the new InsertItem(Index: Integer; const S: string); should call the existing InsertItem(Index: Integer; const S: string; AObject: TObject); with AObject=nil

Bart Broersma

2013-02-08 12:25

developer   ~0065577

> new InsertItem(Index: Integer; const S: string); should call the existing
> InsertItem(Index: Integer; const S: string; AObject: TObject); with
> AObject=nil

I looked at the code for TStringList and wondered why this was not the case there, because the code is the same, except for assinging FObject of course.
I figured it was for some reason I did not understand, so I decided to implement a seperate InsertItem(Index,String).

Back again to the problem.
The InsertItem(Index: Integer; const S: string; AObject: TObject) is actually never called in the first place, I tried with r40197 and a debugln statement in there...

Originally I had the same "sync" code for ItemProps in the new InsertItem there, but that lead to memory leaks (unfreed ItemProps as reported by heaptrace), despite the fact that I added code to free the ItemProps array in the destructor.

To me it looks like an ItemProp is created for an already existing ItemProp, and then we have a "dangling" ItemProp that cannot be reached anymore?

Bart Broersma

2013-02-08 12:31

developer   ~0065578

Would it be easier to store the ItemProps into some sort of TFPList, so we don't have to manipluate the array, but call Insert/Delete/Clear. It might make the code more readable (and therefore easier to maintain?)?

Bart Broersma

2013-02-08 18:20

developer   ~0065586

Found it.
The trick is to restore goAlwaysShowEditor only _after_ FItemProps is updated, or else FItemProps gets out of sync.
Also there is a range check error in Delete(), reading past Length of FItemProps.

Issue History

Date Modified Username Field Change
2013-02-07 20:25 Bart Broersma New Issue
2013-02-07 20:25 Bart Broersma File Added: ValEdit.zip
2013-02-07 20:46 Bart Broersma Note Added: 0065553
2013-02-08 09:20 Jesus Reyes Note Added: 0065571
2013-02-08 12:25 Bart Broersma Note Added: 0065577
2013-02-08 12:31 Bart Broersma Note Added: 0065578
2013-02-08 12:33 Bart Broersma Build => r40197
2013-02-08 18:20 Bart Broersma Note Added: 0065586
2013-02-08 18:30 Bart Broersma Fixed in Revision => r40209
2013-02-08 18:30 Bart Broersma Status new => resolved
2013-02-08 18:30 Bart Broersma Fixed in Version => 1.1 (SVN)
2013-02-08 18:30 Bart Broersma Resolution open => fixed
2013-02-08 18:30 Bart Broersma Assigned To => Bart Broersma
2013-02-11 14:12 Bart Broersma Status resolved => closed