View Issue Details

IDProjectCategoryView StatusLast Update
0003112JEDI VCL00 JVCL Componentspublic2005-07-27 01:07
Reporterchrissi75Assigned Toobones 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.00 
Target VersionFixed in Version3.10 
Summary0003112: TJvFileDirEdit causes application to crash
DescriptionWe've got an OWL application which calls a function from a DLL that creates and shows a VCL dialog modally. That dialog contains a TJvFileDirEdit instance. When destroying the dialog without clicking the browse button the dialog's destructor causes an access violation. I've patched the JvToolEdit unit as follows:

interface

// ...

  TJvFileDirEdit = class(TJvCustomComboEdit)
  public
    // ...
    {$IFDEF VCL}
    destructor Destroy; override;
    {$ENDIF}

// ...

implementation

// ...

{$IFDEF VCL}
destructor TJvFileDirEdit.Destroy;
begin
  FMRUList := nil;
  FHistoryList := nil;
  FFileSystemList := nil;
  FAutoCompleteSourceIntf := nil;
  inherited Destroy;
end;
{$ENDIF}

That changes made the AV disappear.
TagsNo tags attached.

Activities

edbored

2005-07-26 14:24

reporter   ~0007618

Are all of these actually unpopulated? - setting to nil without a free somewhere might leave memory leak...

Would the following not make sense?

{$IFDEF VCL}
destructor TJvFileDirEdit.Destroy;
begin
  if assigned(FMRUList) then
    FMRUList.free;
  FMRUList := nil;

  if assigned(FHistoryList) then
    FHistoryList.free;
  FHistoryList := nil;
 
  if assigned(FFileSystemList) then
    FFileSystemList.free;
  FFileSystemList := nil;

  if assigned(FAutoCompleteSourceIntf) then
    FAutoCompleteSourceIntf.free;
  FAutoCompleteSourceIntf := nil;

  inherited Destroy;
end;
{$ENDIF}

remkobonte

2005-07-26 15:04

developer   ~0007619

FMRUList, FHistoryList, FFileSystemList, FAutoCompleteSourceIntf are all interfaces, so setting them to nil would free the associated objects, if there are no other references to those interfaces.

edbored

2005-07-26 21:05

reporter   ~0007620

Oh of course... That's the problem with replying without access to code to check first.

chrissi75

2005-07-27 00:40

reporter   ~0007621

Besides, the Free methods itself checks for Self <> nil before calling Destroy. Hence it's safe to call Free, even if the object reference used is nil.

obones

2005-07-27 01:07

administrator   ~0007623

Yes, but calling Free on an Interface is very dangerous and leads to "double free" problems. Hence, you simply set it to nil, or let it go out of scope.
I'm not quite sure why setting them to nil is required, maybe the destructors for the implementations of these interfaces do something that requires some other objects to be there.
Anyway, the setting of interfaces to nil is now in CVS, thanks for the report.

Issue History

Date Modified Username Field Change
2005-07-26 01:34 chrissi75 New Issue
2005-07-26 14:24 edbored Note Added: 0007618
2005-07-26 15:04 remkobonte Note Added: 0007619
2005-07-26 21:05 edbored Note Added: 0007620
2005-07-27 00:40 chrissi75 Note Added: 0007621
2005-07-27 01:07 obones Status new => resolved
2005-07-27 01:07 obones Resolution open => fixed
2005-07-27 01:07 obones Assigned To => obones
2005-07-27 01:07 obones Note Added: 0007623