self kaputt? Listen-Initialisierung wegoptimiert? sonstiges?

[DGL-luke]

Hallo,

ich habe eine Klasse, die eine Komponente wrappt und damit unter anderem eine Chain von Mausereignissen ermöglciht:

Delphi-Quellcode:

function TPanelManager.CreateElement(Parent: TWinControl; Visible: Boolean): Boolean;
begin
  Element := TPanel.Create(Parent);
  Element.Parent := Parent;
  Element.Visible := Visible;

  self.MouseDownEvents := TList.Create;
  self.MouseUpEvents := TList.Create;
  self.MouseMoveEvents := TList.Create; //<-!!

  Element.OnMouseDown := PanelMouseDown;
  Element.OnMouseUp := PanelMouseUp;
  Element.OnMouseMove := PanelMouseMove; //<-!!

  Result := true;
end;

procedure TPanelManager.PanelMouseMove(Sender: TObject; Shift: TShiftState; X,
  Y: Integer);
var ListEvent: Pointer;
    xy: TFormPanel;
    c: TControl;
    i: Integer;
begin
  xy := self;

  c := xy.GetElement;

  if xy.MouseMoveEvents.Count > 0 then //(1)
    for i := 0 to xy.MouseMoveEvents.Count-1 do //<-!!
      if Assigned(xy.MouseMoveEvents[i]) then
        TMouseMoveEvent(xy.MouseMoveEvents[i]^)(Sender, Shift, X, Y);
end;

An den markierten ("!!") Stellen sieht man es ja schön - alles initlisiert, es sollte eigentlich nichts schiefgehen.
Aber man sieht ja schon an den etwas wirren Zuweisungen im PanelMouseMove - etwas stimmt doch nicht.

Denn bei "(1)" knallt es - und zwar gewaltig. AV beim Lesen. Warum? Ich zerstöre die Liste nirgends, und vor der Erstellung wird das Event (private-feld in TPanelManager) ja wohl nie ausgelöst. Es knallt schließlich auch, wenn ich mit der Maus über das Control fahre.

TPanelManager ist übrigens von TInterfacedObject abgeleitet und implementiert ein Interface. Er wird auch gerne mal gequeryt, der refcount liegt zur ausführungszeit von PanelMouseMove typischerweise bei 25.

Wo gehen also die Listen verloren?!

[Dax]

Du weist fügst einen Funktionszeiger in die Liste ein, und willst nachher den Funktionszeiger aufrufen, auf den der ursprüngliche Zeiger zeigt?

Kurz: gehts denn, wenn du das ^ weglässt? ;)

[DGL-luke]

ähm.. nein... dann meckert der compiler.

EDIT: eigentlcih will ich ja auch nicht auf den zeiger selbst zuggreifen (seine adresse zur basisiadresse eines methodenaufrufs machen, wenn ich da weit genuug informiert bin), soindern auf seinen wert, deshalb dereferenziere ich ihn.

Speicherinhalt an Adresse des Zeigers = Wert des Zeigers = Adresse des Events!

Im übrigen komme ich gar nicht so weit, bei durchsteppen springt der Compiler in die markierte zeile, sobald ich F9 drücke (also die markeirte zeile ausführe), knallts, sprich, es knallt beim zugriff auf TList(irgendwo).Count.

[negaH]

Ich bin mir nicht sicher ob du weist was du da machen möchtest.

Als wichtigste Info schaue in die Hilfe bei "TMethod" und dessem Aufbau als Record mit 2 Zeigern = 8 Bytes. TMethod ist der Prototyp für alle Events wie OnClick() etc.pp.
Nun frage dich mal was für Datentypen in der TList gespeichert werden, ja es sind Zeiger = 4 Bytes.

Somit kann ein EventTyp mit 8 Bytes nicht direkt in einer TList gespeichert werden, also vermute ich mal das du das Event als dynamischen Record alloziert hast und dann in der TList diesen Zeiger gespeichert hast, weswegen du derefernzierst. Leider sieht man in deinem Source eben nicht was du in die TList speicherst.

Alles in allem habe ich schon einige deiner Sourcen hier in der DP gesehen und mir viel dabei auf das du zu oft mit harten TypCast arbeitest statt mit eigenen Datentypen bzw. mit den richtigen Datentypen zu arbeiten.

Gruß Hagen

[SirThornberry]

auf welche Art und weise schreibst du die Events in die TList? Der Teil fehlt irgendwie. Ich hoffe du schreibst das Event nicht direkt auf die in das TList-Item.

[DGL-luke]

Ähm... noch einmal:

Es knallt bereits BEVOR ich irgendeinen Event aus der Liste hole, geschweige denn ausführe. Es knallt beim Zugriff auf TList.Count, genauer MouseMoveEvents.Count.

@negaH: Harte Casts mach ich in zwei Fällen:

1. Wenn ich in der selben Prozedur bereits einen weichen Cast ausgeführt habe, also weiss, dass ich den richtigen Typen in der Hand habe und mir damit Overhead sparen kann

2. Wenn ich mit TList arbeiten muss, weil ich mir immer noch kein Codeschnipsel für typisierte Listen zurechtgelegt habe *das jetzt gleich mach*

Nur so aus Spaß das Hinzufügen der Methodenzeiger:

Delphi-Quellcode:

function TFormPanel.AddOnMouseMove(Event: TMouseMoveEvent): Integer;
begin
  Result := MouseMoveEvents.Add(@Event);
end;

Falsch oder verkehrt? :mrgreen:

[Hawkeye219]

Einen Zeiger auf eine lokale Variable sollte man nicht dauerhaft speichern...

Gruß Hawkeye

[negaH]

Zitat:

Delphi-Quellcode:

function TFormPanel.AddOnMouseMove(Event: TMouseMoveEvent): Integer; 
begin 
  Result := MouseMoveEvents.Add(@Event); 
end;

Upps erwischt, schau dir in der Hilfe TMethod an, dann vielleicht noch TOnClickEvent, damit du weist was obiges Event als Speicherstuktur bedeutet und dann frage dich worauf der Zeiger der mit @Event referenziert wierd tatsächlich zeigt. (Tipp: was ist ein Stack? und wozu braucht man den ?)

Gruß hagen

[DGL-luke]

Wer wo lokal?

Du wirst mir doch wohl nicht weismachen wollen, dass von Event hier eine lokale Kopie erzeugt wird?! :wall:

Ich ändere das mal geschwind mit ein paar consts... :roll:

@negaH: Warte, da muss ich kurz drüber nachdenken.

EDIT: Aber ändert das irgendwas daran, dass ich auf MouseMoveEvents (die Liste!) offenbar immer noch nicht zugreifen kann bzw. dass ich keinen gütligen Zeiger drauf hab?

EDIT: Aber ein TMouseMoveEvent ist doch nur eine procedure of object... ist das dann = TMethod? und wo kommt dann eigentlich self her, ist das code oder data?

Fragen über Fragen... aber danke, dass ihr euch alle so Mühe gebt mit mir :)

[SirThornberry]

völlig falsch!! Du addest in die Liste die Adresse wo die Variable auf dem Stack liegt. Nach verlassen der Methode liegt die Methode dann aber nicht mehr auf dem Stack und somit zeigt dein Zeiger auf was ganz anderes.

[DGL-luke]

ok, SirT, das leuchtet mir ein... ändert ein const da dran was? bzw. was SOLL ich ändern?

[negaH]

Zitat:

völlig falsch!! Du addest in die Liste die Adresse wo die Variable auf dem Stack liegt. Nach verlassen der Methode liegt die Methode dann aber nicht mehr auf dem Stack und somit zeigt dein Zeiger auf was ganz anderes.

Nicht gaaaanz richtig ;)

Die Adresse die auf die Variable im Stack zeigt, zeigt weiterhin nach Verlassen der Funktion auf den Stack. ABER die Daten auf diesem Stack werden durch nachfolgendem Code permanent mit neuen Daten überschrieben. Das bedeutet das bei dem dereferenzierten Aufruf (und TypCast klar) man Daten auf dem Stack als Einsprungadresse in eine Methode interpretiert die irgendwo hin aber nicht dahin wo sie es sollte, zeigt. Der Programcounter der CPU springt also an irgendeine Addresse im Addressraum des Speichers.

Gruß Hagen

[SirThornberry]

hatte ich doch geschrieben das der zeiger dann auf was anderes zeigt (also auf andere daten). Aber das ist OT - zurück zum Thema.

@Luke: Du solltest das Event kopieren. Und da ein Event größer als ein einfacher Pointer ist musst du dafür speicher anfordern (mit new) und diesen dann in deine Liste hängen.

[DGL-luke]

ok, werd ich machen.

Mit einer typed TObjectList, wie es sie in der Codelib gibt, funzt die sache offenbar auch nciht - er hält das event in FList.Add(_item) für einen Methodenaufruf...

EDIT: So?:

Delphi-Quellcode:

function TFormPanel.AddOnMouseMove(const Event: TMouseMoveEvent): Integer;
var CopyEvent: Pointer;
begin
  CopyEvent  := GetMemory(sizeof(TMouseMoveEvent));
  CopyMemory(CopyEvent,@Event, sizeof(TMouseMoveEvent));
  Result := MouseMoveEvents.Add(CopyEvent);
end;

Das is aber n bisschen umständlich, scheint mir... und was ist jetzt mit dem const? hat das irgendeinen einfluss auf irgendwas?

Und vor allem: Das löst nicht das ursprungsproblem...

[Hawkeye219]

Die Alternative wäre ein dynamisches Array, du mußt es ja nicht unbedingt in 1er-Schritten vergrößern. Damit reduzierst du aber den Verwaltungsaufwand beim Anfordern/Freigeben der Elemente.

Gruß Hawkeye

[DGL-luke]

Jaaa... und was ist jetzt mir meiner anfänglichen Frage? :wall:

Die existiert immer noch, denn fakt ist, wenn ich auf MouseMoveList.Count zugreife, gibt es eine AV... :cry:

EDIT: nur zur auffrischung:

Delphi-Quellcode:

function TPanelManager.AddOnMouseMove(const Event: TMouseMoveEvent): Integer;
var CopyEvent: Pointer;
begin
  CopyEvent  := GetMemory(sizeof(TMouseMoveEvent));
  CopyMemory(CopyEvent,@Event, sizeof(TMouseMoveEvent));
  Result := MouseMoveEvents.Add(CopyEvent);
end;

So adde ich ein Event zur Liste - stimmt das jetzt so?

Und hier ist der schadcode:

Delphi-Quellcode:

procedure TPanelManager.PanelMouseMove(Sender: TObject; Shift: TShiftState; X,
  Y: Integer);
var ListEvent: Pointer;
    xy: TFormPanel;
    c: TControl;
    i: Integer;
begin
  xy := self;

  c := xy.GetElement;

  if xy.MouseMoveEvents.Count > 0 then  //<<-- KNALLT HIER
    for i := 0 to xy.MouseMoveEvents.Count-1 do
      if Assigned(xy.MouseMoveEvents[i]) then
        TMouseMoveEvent(xy.MouseMoveEvents[i]^)(Sender, Shift, X, Y);
end;

Wos knallt hab ich hoffentlich fett genug markiert...

EDIT: Den TPanelManager bekommt die Anwendung nur einmal in die Hand, dabei wird er in einen InterfaceManager created, der von da an das Interface bereitstellt.
Das PanelMouseMove und die TList SOLLTEN aber davon doch absolut unbeeinflusst bleiben, es funktioniert ja offenbar auch der self-pointer - obwohl ich das einmal überprüfen sollte.
Kann es denn sein, dass mir da irgendwelche Adressen/Instanzen verschütt gehen?

[negaH]

Ok, du erzeugst deine TList's nur im Constructor und zerstörst sie nur im Destructor vom TPanelManager, an keiner anderen Stelle im Source, richtig ?

Dann ändere in deinem Destructor vom TPanelManager das Freigeben der Listen so ab das du FreeAndNil() dafür benutzt. Wichtig ist nur das du deine TList Felder auf NIL setzt.

Jetzt debuggst du nochmal und prüfst Self.MouseMoveEvents <> nil ab. Ich vermute nämlich das du durch das Mixen von Interfaces und Objecten konzeptionell durcheinander kommst und das dein TPanelManager Object beim Aufruf von OnMouseMove() irgendwie zerstört wurde. Auf alle Fälle wäre das die einzigste Erklärung für einen logischen Programmierfehler, mit deinem vorgstellten wenigen Sourcen, die mir einfällt.

Die wesentlich schlechtere Vermutung wäre das du tatsächlich mit wildem TypCasting Spiecherdaten überschreibst, das liegt aber dann eher am Programmierstil ansich.

Ach und nochwas:
Deine Zählschleife "for I := 0 to Liste.Count -1" ändere diese in eine downto Schleife ab. Immerhin rufst du eine Ereignissmethode auf und diese könnte sich ja exakt in ihrem Aufruf beim TPanelManager deinstallieren, also aus der TList selber entfernen. Das führt dann in deiner Schleife zum nächsten Crash.

Und nochwas:
Statt die Events in TLists zu speichern, baue dir lieber was eigenes. Entweder ein dynamisches Array of TMethod, was ich am sinnvollsten erachte oder eigene Records die verlinkt untereinander sind (verlinkte Liste) oder eben ein eigenes TList Object. Wichtig ist nur das du dann OHNE Zeiger, Speicherkopierungen etc.pp. auskommen wirst und mit simplem PASCAL Zuweisungen arbeiten kannst. Das reduziert enorm die Fehleranfälligkeit im Programcode gegen logische Programmierfehler. Der Fehler mit der Referenzierung des Events das auf dem Stack liegt IST defakto ein logischer Programmierfehler, du warst dir garnicht im Klaren was du da gemacht hast. Ergo ist es besser auf simple Standard-Konstrukte des PASCALs zurückzugreifen und eben Zeigerarithmetiken, TypCast und mischen von Objekten und Interfaces zu verzichten.

Ein beliebiges Event kannst du so sicher casten:

Delphi-Quellcode:

procedure XYZ(const Event: TMouseMoveEvent);
var
  MyEvent: TMethod;
begin
  MyEvent.Data := TMethod(Event).Data;  // <- Self des Eigners des Events aud den Code zeigt
  MyEvent.Code := TMethod(Event).Code;  // <- Code ist die Addresse der Methode des Eigners die aufgerufen werden soll.
end;

Gruß Hagen

[DGL-luke]

Danke negaH, endlich ein Lichtblick... ja, beim debuggen habe ich mich im debug-inspektor mal bis zu TInterfacedObject.FRefCount durchgeklickt - und der war 0?! Ich seh mir das mal an...

"wildes" rumgecaste hab ich jetzt eigentlich keins mehr. Aber ich werde mal eine TMethodList komplett selberbauen, dann habe ich auch die andauernde Unterschioedung zwischen TMouseEvent und TMouseMoveEvent weg.

Zitat:

Ok, du erzeugst deine TList's nur im Constructor und zerstörst sie nur im Destructor vom TPanelManager, an keiner anderen Stelle im Source, richtig ?

Mmmm... nicht ganz :)
Erzeugt werden sie im Aufruf von TPanelManager.CreateObject, das sollte ich irgendwo schon gepostet haben. Und freigegeben... bis jetzt... noch nie...

verdammt, ich muss off. ich schreib später weiter :roll:

EDIT: Alsoooo....

Ich hatte zwischendurch Angst, dass der Konstruktor nicht aufgerufen wird, sondern der von einer Basisklasse...

(glaube den Code kann ich zeigen)

Delphi-Quellcode:

//Initialisierung:
with Toolbox.Categories.Add do
        begin
          Caption := 'Containers';
          with Items.Add do
            begin
              Caption := 'Panel';
              Data := Pointer(PageManager.Supplier.AddClass(TFormPanel));
            end;
        end;

function TFormElementSupplier.AddClass(EClass: TFormElementClass): Integer;
begin
  if not Supports(EClass,IID_IFormElement) then
    Raise EElementNotMatching.Create('This class does not support IFormElement interface!'); 
  
  Result := Classes.Add(EClass); //TClassList
end;

function TFormElementSupplier.Instantiate(EClassIndex: Integer): IFormElement;
var EClass: TFormElementClass;
    Element: TInterfacedObject;
    ElementIndex: Integer;
begin
  EClass := TFormElementClass(Classes[EClassIndex]);

  Element := EClass.Create as TInterfacedObject; //fragliche Stelle

  ElementIndex := Elements.Add(Element);

  Result := Element as IFormElement;
end;

So wird das erzeugt.... wird an //fraglicher stelle denn der richtige Konstruktor aufgerufen? override konnte ich ja nicht machen, da ist drunter wohl nur TObject.Create, und das ist nicht virtuell...

kurz das grundgerüst meines TPanelManager:

Delphi-Quellcode:

type
  TPanelManager = class(TInterfacedObject, IFormElement);
    //IFormElement
      *implementier*
    constructor Create; //override; // geht nicht
  end;

also ich denke mal, da wird auf jeden fall TObject.Create aufgerufen. Wäre es da nicht am praktikabelsten, eine Init-Prozedur ins Interface zu packen, die ich im Supplier aufrufe?

[negaH]

Zitat:

"wildes" rumgecaste hab ich jetzt eigentlich keins mehr. Aber ich werde mal eine TMethodList komplett selberbauen, dann habe ich auch die andauernde Unterschioedung zwischen TMouseEvent und TMouseMoveEvent weg.

Stop, die Unterschiede zwischen diesen beiden Event-Typen liegen wahrscheinlich in deren Parametern. Sie sind also eben NICHT gleich und sie in einer einzigsten Liste zu speichern ist unsinnig. Denn beim Aufruf dieser Events aus der List musst du sie ja so casten das sie die korrekte Parametersignatur aufweisen, ansonsten kracht es gewaltig.

Gruß Hagen

[DGL-luke]

:lol:

Na klar, denkst du ich bin lebensmüde? Ich sprache ja auch von TMethodList ;)

(ich hab übrigens oben editiert.)

[DGL-luke]

Schuster bleib bei deinem Leisten....

Zitat:

Zitat von 3_of_8

Ich würde da eine abstrakte Klasse hernehmen

Ja, 15 "virtual; abstract;"s und 15 "override;" sowie ein paar Textersetzungen später hat dann auch alles funktioniert. Gut funktioniert.

Naja, hat halt mit Interface einfach nicht sollen sein. werde die Testimplementation (TPanelManager) aus der Basis-Unit rausnehmen, und dann passt das.

Für mich mal wieder: Thema ergebnislos erledigt... :(

Ich werde mal den Chef fragen, ob ich die TEventList veröffentlichen darf.