Unerklärliche EAccessViolation -> Meldung Invalid Pointer Operation

[FragenderHerbert]

Hallo,

ich habe gerade einen Dialog in Arbeit, aus dem ich später vorher registrierte Tools auswählen will. Jedoch erhalte ich die im Titel genannte Exception, die ich mir nicht erklären kann.

Es geht um einen Auswahldialog, aus dem ich ein vorher registriertes Kommandozeilentool als aktuell aktives auswählen will. Doch dieser Dialog wirft die genannte Exception bei AssignCommands im Hauptprogramm:

Delphi-Quellcode:

procedure TMainForm.menuActiveToolClick(Sender: TObject);
var
  CurrentTool: Integer;
begin
  if Assigned(CmdLines) and (CmdLines.Count > 0) then
  begin
    if Assigned(DlgChooseCommandTool) then
      DlgChooseCommandTool.AssignCommands(CmdLines); //Hier kommt die Exception
    DlgChooseCommandTool.ShowModal;
    if DlgChooseCommandTool.ModalResult = mrOk then
    begin
      CurrentTool := DlgChooseCommandTool.ChoosedIndex;
      ShowMessage('Index Of Current Tool is: '+IntToStr(CurrentTool));
    end;
  end
  else ShowMessage('Bitte registrieren Sie zuerst ein Tool im Menü [Tool registrieren...]!');
end;

Der Delphi Debugger springt in diese Systemroutine -> _LStrArrayClr(var StrArray; cnt: longint) an diese Stelle:

Delphi-Quellcode:

        CALL    _FreeMem
@@doneEntry:

Hier ist der Auswahldialog:

Delphi-Quellcode:

unit UDlgChooseCommandTool;

interface

uses
  Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
  Dialogs, StdCtrls, UCommandInterface, UCmdTools;

type
  TCommands = class(TInterfacedObject, ICommandTool)
  private
    FCommands: TCmdTools;
  public
    constructor Create;
    destructor Destroy; override;

    function GetCommand(Index: Integer): TCmdTool;
    function GetCount: Integer;


    procedure AssignCommands(Commands: TCmdTools);
    procedure RegisterCommand(
      Name, Command: String; Options:String=''; CfgFile:String=''; AOptionsFactory:TOptionsFactory=nil
    );
    procedure SetCommand(Index: Integer; Value: TCmdTool);

    property Command[Index: Integer]: TCmdTool read GetCommand;
    property Count: Integer read GetCount;
  end;

  TDlgChooseCommandTool = class(TForm)
    lbxChoosedCommandTool: TListBox;
    cbxChoosedCommandTool: TComboBox;
    lbRegisteredCommandTools: TLabel;
    lbChoosedCommandTool: TLabel;
    btnOk: TButton;
    btnCancel: TButton;
    btnHelp: TButton;
    procedure FormCreate(Sender: TObject);
    procedure lbxChoosedCommandToolClick(Sender: TObject);
    procedure btnOkClick(Sender: TObject);
    procedure FormDestroy(Sender: TObject);
  private
    { Private declarations }
    FCommands: TCommands;
    FCurrentChoosed: Integer;
    //FOptionsFactory: TComponent;
    function GetChoosedIndex: Integer;
  public
    { Public declarations }
    procedure AssignCommands(Commands: TCmdTools);
    property ChoosedIndex: Integer read GetChoosedIndex;
  end;

var
  DlgChooseCommandTool: TDlgChooseCommandTool;


implementation

{$R *.dfm}

procedure TDlgChooseCommandTool.AssignCommands(Commands: TCmdTools);
var Index: Integer;
begin
  if (Assigned(Commands)) and (Assigned(FCommands)) then
  begin
    FCommands.AssignCommands(Commands);
    for Index := 0 to Commands.Count - 1 do
    begin
      cbxChoosedCommandTool.Items.Add(FCommands.Command[Index].Name);
      lbxChoosedCommandTool.Items.Add(FCommands.Command[Index].Name);
    end;
  end; 
end;

procedure TDlgChooseCommandTool.btnOkClick(Sender: TObject);
begin
  GetChoosedIndex;
end;

procedure TDlgChooseCommandTool.FormCreate(Sender: TObject);
begin
  FCommands := TCommands.Create;
end;

procedure TDlgChooseCommandTool.FormDestroy(Sender: TObject);
begin
  if Assigned(FCommands) then begin FCommands.Free; FCommands := nil; end;
end;

function TDlgChooseCommandTool.GetChoosedIndex: Integer;
var Index: Integer;
begin
  Result := -1;
  while Index < lbxChoosedCommandTool.Items.Count do
  begin
    if cbxChoosedCommandTool.Items[Index] = cbxChoosedCommandTool.Text then
    begin
      FCurrentChoosed := Index;
      Result := Index;
      Index := lbxChoosedCommandTool.Items.Count;
    end;
    Inc(Index);
  end;
end;

procedure TDlgChooseCommandTool.lbxChoosedCommandToolClick(Sender: TObject);
begin
  cbxChoosedCommandTool.ItemIndex := lbxChoosedCommandTool.ItemIndex;
  cbxChoosedCommandTool.Text := cbxChoosedCommandTool.Items[cbxChoosedCommandTool.ItemIndex];
end;

{ TCommands }

procedure TCommands.AssignCommands(Commands: TCmdTools);
begin
  if Assigned(Commands) then FCommands.Assign(Commands);
end;

constructor TCommands.Create;
begin
  inherited Create;
  FCommands := TCmdTools.Create;
end;

destructor TCommands.Destroy;
begin
  FCommands.Free;
  inherited;
end;

function TCommands.GetCommand(Index: Integer): TCmdTool;
begin
  if Assigned(FCommands) then
   Result := TCmdTool(FCommands[Index])
  else Result := nil;
end;

function TCommands.GetCount: Integer;
begin
  Result := FCommands.Count;
end;

procedure TCommands.RegisterCommand(Name, Command, Options, CfgFile: String;
  AOptionsFactory: TOptionsFactory);
begin
  FCommands.AddCmdTool(RegisterCommandLineTool(Name, Command, Options, CfgFile, AOptionsFactory));
end;

procedure TCommands.SetCommand(Index: Integer; Value: TCmdTool);
begin
  {
  FCommands.CmdTool[Index].Name := Value.Name;
  FCommands.CmdTool[Index].Command := Value.Command;
  FCommands.CmdTool[Index].CfgFile := Value.CfgFile;
  }
  FCommands.CmdTool[Index].Factory := Value.Factory;
end;


end.

Diese Unit stellt mein Kommandotoolinterface bereit:

Delphi-Quellcode:

unit UCmdTools;

interface

uses
  Classes, Contnrs, IniFiles;

type
  //Platzhalter für späteren Optionsdialog
  //zur interaktiven Einstellung der Kom-
  //mandozeilenparameter des aktuell ausge-
  //wählten Tools
  TOptionsFactory = TComponent;

  //ein Kommandozeilentool
  TCmdTool = class(TObject)
  private
    FCfgFile: String;
    FCommand: String;
    FName: String;
    FOptions: String;
    FOptionsFactory: TOptionsFactory;
    function GetOptionsFactory: TOptionsFactory;
    procedure SetOptionsFactory(const Value: TOptionsFactory);
  public
    procedure ApplyOptions; virtual; abstract;
    constructor Create(aName,aCommand: String; aOptions:String=''; aCfgFile:String='');
    destructor Destroy; override;
    function CfgFile: String; //cfg Datei für Kommandozeilenparameter
    function Command: String; //exename + Optionen
    function Name: String;    //Name im Menü
    property Factory: TOptionsFactory read GetOptionsFactory write SetOptionsFactory;
  end;

  //Liste aller registrierten Tools
  TCmdTools = class(TObjectList)
    function GetCmdTool(Index: Integer): TCmdTool;
    function AddCmdTool(CmdTool: TCmdTool): Integer;
    property CmdTool[Index: Integer]: TCmdTool read GetCmdTool;
  end;

var
  CmdLines: TCmdTools;

//Diese Funktion soll ein Tool registrieren (in die Liste schreiben)
function RegisterCommandLineTool(Name, Command: String; Options:String=''; CfgFile:String=''; AOptionsFactory:TOptionsFactory=nil): TCmdTool;

implementation

function RegisterCommandLineTool(Name, Command: String; Options:String=''; CfgFile:String=''; AOptionsFactory:TOptionsFactory=nil): TCmdTool;
var CmdTool: TCmdTool;
begin
  try
    CmdTool := TCmdTool.Create(Name, Command, Options, CfgFile);
    CmdTool.Factory := AOptionsFactory;
    CmdLines.AddCmdTool(CmdTool)
  finally
    Result := CmdTool;
    CmdTool.Free;
  end;
end;

{ TCmdTools }

function TCmdTools.AddCmdTool(CmdTool: TCmdTool): Integer;
begin
  Result := Add(CmdTool);
end;

function TCmdTools.GetCmdTool(Index: Integer): TCmdTool;
begin
  Result := TCmdTool(Items[Index])
end;

{ TCmdTool }

function MakeOptFile(AOptions: String): String;
var f: file; opts: array[0..127] of char; w:longint;
begin
  fillchar(opts, Sizeof(opts), ' ');
  move(AOptions[1], opts, Sizeof(opts));
  Assign(f, 'extrafpc.cfg');
  Rewrite(f);
  blockwrite(f, opts, Sizeof(opts), w);
  Close(f);
  MakeOptFile := '@extrafpc.cfg';
end;

function TCmdTool.CfgFile: String;
begin
  Result := FCfgFile;
end;

function TCmdTool.Command: String;
begin
  if FCommand <> '' then
  if FOptions <> '' then
    Result := FCommand + ' ' + FCfgFile + ' ' + FOptions;
end;

constructor TCmdTool.Create(aName,aCommand: String; aOptions:String=''; aCfgFile:String='');
begin
  FCfgFile := CfgFile;
  FOptions := FOptions;
  FCommand := Command;
  FName    := Name;
end;

destructor TCmdTool.Destroy;
begin
  if Assigned(FOptionsFactory) then
  begin
    FOptionsFactory.Free;
    FOptionsFactory := nil;
  end;
  inherited;
end;

function TCmdTool.GetOptionsFactory: TOptionsFactory;
begin
  Result := FOptionsFactory;
end;

function TCmdTool.Name: String;
begin
  Result := FName;
end;

procedure TCmdTool.SetOptionsFactory(const Value: TOptionsFactory);
begin
  if FOptionsFactory <> Value then
  begin
    if Assigned(FOptionsFactory) then
    begin
      FOptionsFactory.Free;
      FOptionsFactory := nil;
    end;
    FOptionsFactory := Value;
  end;
end;

initialization
  CmdLines := TCmdTools.Create;

finalization
  CmdLines.Free;

end.

[jaenicke]

Mache aus der globalen Variablen DlgChooseCommandTool einmal eine lokale Variable.

Delphi-Quellcode:

procedure TMainForm.menuActiveToolClick(Sender: TObject);
var
  CurrentTool: Integer;
  DlgChooseCommandTool: TDlgChooseCommandTool;
begin
  if Assigned(CmdLines) and (CmdLines.Count > 0) then
  begin
    DlgChooseCommandTool := TDlgChooseCommandTool.Create(nil);
    try
      DlgChooseCommandTool.AssignCommands(CmdLines); //Hier kommt die Exception
      if IsPositiveResult(DlgChooseCommandTool.ShowModal) then
      begin
        CurrentTool := DlgChooseCommandTool.ChoosedIndex;
        ShowMessage('Index Of Current Tool is: '+IntToStr(CurrentTool));
      end;
    finally
      DlgChooseCommandTool.Free;
    end;
  end
  else ShowMessage('Bitte registrieren Sie zuerst ein Tool im Menü [Tool registrieren...]!');
end;

Passiert es dann immer noch?

Genauso ist es sinnvoll globale Variablen wie CmdLines generell zu vermeiden.

[himitsu]

Zitat:

Delphi-Quellcode:

function RegisterCommandLineTool(Name, Command: String; Options:String=''; CfgFile:String=''; AOptionsFactory:TOptionsFactory=nil): TCmdTool;
var CmdTool: TCmdTool;
begin
  try
    CmdTool := TCmdTool.Create(Name, Command, Options, CfgFile);
    CmdTool.Factory := AOptionsFactory;
    CmdLines.AddCmdTool(CmdTool)
  finally
    Result := CmdTool;
    CmdTool.Free;   <<<<<<<<<< o_O
  end;
end;

[FragenderHerbert]

Hallo,

danke erst mal für eure schnelle Antwort.

@jaenicke: Nein, nach dieser Änderung kommt die Exception nicht mehr.

@himitsu: Ok, hab diese Zeile auskommentiert, habe dazu allerdings eine Verständnisfrage, die sich auf Zeiger-Alise bezieht. Ist nicht der Adresswert eh in der Liste Cmdlines, in die ich CmdTool eingefügt habe? (CmdLines.AddCmdTool)?
Warum muss ich dann noch diese lokale Variable behalten? Kann ich denn sicher sein, das nach Beenden der Funktion diese Variable korrekt frei gegeben wird?

[Sir Rufo]

Du zerstörst damit die Instanz, die du aber auch zurückgibst.

Das ist ungefähr so, als ob ich dir ein Auto verkaufe (Instanz) und dir den Brief (Referenz) aushändige.
Dann schiebe ich das Auto in die Presse (Instanz zerstören). Du hast zwar noch den Brief (Referenz), aber mit dem Auto kannst du nicht mehr fahren
Wenn du es trotzdem versuchst, fällst du auf die Nase (Access Violation)

[Union]

Sir Rufo, damit solltest Du öffentlich auftreten

[baumina]

Ich mag anschauliche Erklärungen. *schnell mal nachschauen, ob mein Auto noch draußen steht*

[FragenderHerbert]

Ok, dake soweit. Da liegt wohl mein Denkfehler darn, das ich, um bei der Analogie zu bleiben, mit
 CmdLine.AddCmd(CmdTool); mein Auto namens CmdTool schon in der Grage hatte. Ich hab geglaubt, die Lokale Variable CmdTool sei nur ein Platzhalter, der Autoransporter in der Analogie. Der könnte ja ein weiteres Auto holen und in die Garage stellen, wider mit hier CmdLine.AddCmd(Cmdtool);

Wie htte ich das dann anders schreiben müssen, damit meine lokale Variable wirklich nur dieser Autotransporter ist?

(Jetzt will ich das genau wissen, damit so ein Fehler nicht wieder passiert)

[jaenicke]

Da du das Objekt danach noch benötigst, kannst du es an der Stelle gar nicht zerstören. Am einfachsten kannst du daher gleich das schreiben:

Delphi-Quellcode:

function RegisterCommandLineTool(const AName, ACommand: String; const AOptions: String = ''; const ACfgFile: String = '';
  AOptionsFactory: TOptionsFactory = nil): TCmdTool;
begin
  Result := TCmdTool.Create(AName, ACommand, AOptions, ACfgFile);
  Result.Factory := AOptionsFactory;
  CmdLines.AddCmdTool(Result);
end;

Und CmdLines würde ich mit dem Parameter True an den Konstruktor erzeugen, damit die darin gespeicherten Objekte beim Freigeben der Liste mit freigegeben werden:

Delphi-Quellcode:

initialization
  CmdLines := TCmdTools.Create(True);

[Sir Rufo]

Der Punkt ist, es wird niemals eine Instanz transportiert.

Eine Variable zeigt nur auf den Ort, wo sich die Instanz befindet (Referenz auf die Speicherstelle).
Stell dir das als Zettel vor, wo du dir eine Adresse merkst. Diesen Zettel kannst du kopieren und weitergeben, die Instanz wird dadurch nicht mehr (Visitenkarten fangen auch nicht an mich zu klonen, wenn ich nur genug davon verteile).

Wird die Instanz zerstört, dann wird der Speicherbereich freigegeben, den die Instanz belegt.