Abfragen ob eine Variable oder die Referenz noch existiert

[Cyberaxx]

Hallo,

hoffe der Titel ist für das Problem was ich habe richtig gewählt.
Das Try / Except ist für mich jetzt nur eine Lösung, damit das Programm nicht ständig hängen bleibt.

Delphi-Quellcode:

    try

      while (Pos(EOCommand, AClient.Buffer) > 0) do begin
        Index := Pos(EOCommand, AClient.Buffer);
        DCFrame := Copy(AClient.Buffer, 1, Index + 4); 
        AClient.Buffer := Copy(AClient.Buffer, Index + 5, Length(AClient.Buffer)); 

        Parse_DCFrame(AClient, DCFrame);
        end;

    except
      Exit;
    end;

AClient enthällt einen Socket und einen String;
Das klappt alles ohne Probleme solange nicht versucht wird das sich ein Benutzer doppelt einloggt.
Meldet sich ein Benutzer ein zweites mal an wird in der Procedure OnClientDisconnet AClient freigegeben,
danach springt er wieder in die Schleife und da Existiert "AClient" eigentlich nicht mehr, der Debugger sagt da so schön "nicht verfügbarer Wert" zu. Kann man irgendwie darauf Prüfen oder ex mit einer besseren Lösung als einem Try / Except abzufangen?
Würde ungerne alles umbauen damit ich ihn nil setzen kann, das wäre so ein wenig zu viel was ich ändern müsste.

Hilfe :)

Gruß

Dsn

[Klaus01]

Guten Morgen,

kannst Du nicht AClient auf nil überprüfen?
Oder AClient mit Assigned abfragen?

Delphi-Quellcode:

if Assigned(AClient) then

Grüße
Klaus

[Cyberaxx]

Delphi-Quellcode:

    try

      while (Pos(EOCommand, AClient.Buffer) > 0) do begin
        Index := Pos(EOCommand, AClient.Buffer);
        DCFrame := Copy(AClient.Buffer, 1, Index + 4); // +4 ersetzten durch Length(EOCommand)
        AClient.Buffer := Copy(AClient.Buffer, Index + 5, Length(AClient.Buffer)); // +5 ersetzen

        Parse_DCFrame(AClient, DCFrame);
        end;

    except
      Exit;
    end;

Problem ist wie baue ich es in diese Schleife mit ein.
Im Schleifenkopf geht es nicht da er dort ja auch gleich auf den String zugreift.

ich könnte es aber mal so testen

Delphi-Quellcode:

    try

      while assigned(AClient) do begin
        if (Pos(EOCommand, AClient.Buffer) = 0) then
          Exit;

        Index := Pos(EOCommand, AClient.Buffer);
        DCFrame := Copy(AClient.Buffer, 1, Index + 4); // +4 ersetzten durch Length(EOCommand)
        AClient.Buffer := Copy(AClient.Buffer, Index + 5, Length(AClient.Buffer)); // +5 ersetzen

        Parse_DCFrame(AClient, DCFrame);
        end;

    except
      Exit;
    end;

Falls das nicht klappt, wüsste ich aber auch icht weiter auf anhieb.

[Klaus01]

Vielleicht so:

Delphi-Quellcode:

  if Assigned(AClient) then
    begin
      while (Pos(EOCommand, AClient.Buffer) > 0) do begin 
        Index := Pos(EOCommand, AClient.Buffer); 
        DCFrame := Copy(AClient.Buffer, 1, Index + 4); 
        AClient.Buffer := Copy(AClient.Buffer, Index + 5, Length(AClient.Buffer)); 

        Parse_DCFrame(AClient, DCFrame); 
        end; 
    end;

oder so:

Delphi-Quellcode:

      while ((Assigned(AClient)) AND(Pos(EOCommand, AClient.Buffer) > 0)) do begin 
        Index := Pos(EOCommand, AClient.Buffer); 
        DCFrame := Copy(AClient.Buffer, 1, Index + 4); 
        AClient.Buffer := Copy(AClient.Buffer, Index + 5, Length(AClient.Buffer)); 

        Parse_DCFrame(AClient, DCFrame); 
        end;

Grüße
Klaus

[Cyberaxx]

Deine Erste Methode würde glaube ich nicht viel Bringen da er nur beim ersten mal Prüft ob der AClient zugewisen ist.
Die untere Methode habe ich auch ausprobiert klappt ebenfalls nicht. Habe noch etwas anderes versucht und dann brauch ich dazu mal eine erklärung bitte wie das zustande kommt.

Delphi-Quellcode:

    if AClient.Buffer > '' then begin
      while (Pos(EOCommand, AClient.Buffer) > 0) do begin
        Index := Pos(EOCommand, AClient.Buffer);
        DCFrame := Copy(AClient.Buffer, 1, Index + 4); // +4 ersetzten durch Length(EOCommand)
        AClient.Buffer := Copy(AClient.Buffer, Index + 5, Length(AClient.Buffer)); // +5 ersetzen

        Parse_DCFrame(AClient, DCFrame);

        if not Assigned(AClient) then
          Exit;

        if AClient.Buffer = '' then
          Exit;
        end;

Bin das mit dem Debugger durchgegangen.
Wenn ich über Assigned(AClient) fahre steht dort -> Nicht verfügbarer Wert
Im Debug-Insprektor jedoch sind noch alle Daten vorhanden.

Versuche ich das ganze mit AClient.Buffer ->

Zitat:

---------------------------
Fehler
---------------------------
Fehler beim Untersuchen von 'AClient.Buffer': Nicht verfügbarer Wert
---------------------------
OK
---------------------------

Lasse ich es weiter durchlaufen springt er wieder in die Schleife und erst dann kommt die Exception

[Muetze1]

Delphi-Quellcode:

try

      while assigned(AClient) and (Pos(EOCommand, AClient.Buffer) > 0) do begin
        Index := Pos(EOCommand, AClient.Buffer);
        DCFrame := Copy(AClient.Buffer, 1, Index + 4);
        AClient.Buffer := Copy(AClient.Buffer, Index + 5, Length(AClient.Buffer));

        Parse_DCFrame(AClient, DCFrame);
        end;

    except
      Exit;
    end;

Zitat:

Zitat von Cyberaxx

Meldet sich ein Benutzer ein zweites mal an wird in der Procedure OnClientDisconnet AClient freigegeben, ...

Sorge dafür, dass FreeAndNil() genutzt wird, damit die Abfrage klappt. Ausserdem gehe ich davon aus, dass AClient die gleiche Variable ist wie in dem Code zuvor (nicht Typgleich sondern wirklich die gleiche Variable).

[Cyberaxx]

AClient ist in dem Fall nur ein Item aus einer Collection.

Wenn ich sie auf nil setze müsste sie doch auch nachdem der Socket getrennt wurde noch vorhanden sein oder verstehe ich das da etwas falsch?

Delphi-Quellcode:

procedure TDC_SPA.FSocketClientRead(Sender: TObject;
  Socket: TCustomWinSocket);
  var
    Index: Integer;
    AClient: TClient;
    DCFrame: String;
begin
  if FClients.FindSocket(Socket, Index) then begin
    AClient := FClients.Client[Index];

    AClient.Buffer := AClient.Buffer + Socket.ReceiveText;

    AClient.Buffer := StringReplace(AClient.Buffer, #$d#$a, LineFeed, [rfReplaceAll]);


    if AClient.Buffer > '' then begin
      while (Pos(EOCommand, AClient.Buffer) > 0) do begin
        Index := Pos(EOCommand, AClient.Buffer);
        DCFrame := Copy(AClient.Buffer, 1, Index + 4); // +4 ersetzten durch Length(EOCommand)
        AClient.Buffer := Copy(AClient.Buffer, Index + 5, Length(AClient.Buffer)); // +5 ersetzen

        Parse_DCFrame(AClient, DCFrame);

        if not Assigned(AClient) then
          Exit;

        if AClient.Buffer = '' then
          Exit;

        if AClient.SID = 0 then
          Exit;
        end;

      end;
    end;

end;

procedure TDC_SPA.Parse_DCFrame(AClient: TClient; AFrame: String);
  var
    Framelist: TFramelist;
    Command: Integer;
    AUser: TUser;
    Index: Integer;
begin
  if Assigned(FOnRawData) then
    FOnRawData(Self, rsIn, AClient.SID, AFrame);

  [...]

  case Command of // Kommandoframe auswerten
    CMD_AUTH: HandleLogin(AClient, Framelist); // Anmelden
    CMD_PONG: HandlePong(AClient, Framelist); // Pong
    CMD_SEND_USER: HandleUserMessage(AClient, FrameList); // Nachricht an einen Benutzer

    // Alle Commands > 100 werden extern gehandled;
    else begin
    // Sicherstellen das das Command über 100 liegt
      if Command < 100 then
        Exit;

      // Nur wenn der Socket einem Benutzer zugeordnet werden kann fortfahren
      if Users.FindSocket(AClient.Socket, Index) then
        AUser := Users.User[Index]
          else Exit;

      // Übergabe an Extern
      if Assigned(FOnCommand) then
        FOnCommand(Self, AUser, Command, Framelist);
      end;
  end;

  Framelist.Free;
end;

procedure TDC_SPA.HandleLogin(AClient: TClient; FrameList: TFrameList);
  var
    Index: Integer;
    AUser: TUser;
    AuthVal: Boolean;
begin
  if Framelist.Count <> 6 then // Anzuahl der Frames MUSS 6 sein
    begin
    SendMessage(AClient.Socket, CMD_LOGIN_FAIL, 'Login Error. Not enough parameters'); // Fehlermeldung falls ungültig
    AClient.Socket.Close; // Socket freigeben
    Exit;
    end;

  if Not Users.FindUser( FrameList.Frames[1].Frame, Index ) then begin
    AUser := FUsers.AddConnection(AClient.Socket);
    AUser.SID := AClient.SID;
    AUser.LoginTime := Now;
    AUser.LPStamp := TimeStamp;
    AUser.Address := FrameList.Frames[3].Frame;
    AUser.User := FrameList.Frames[1].Frame;
    AUser.Name := FrameList.Frames[2].Frame;
    AUser.Pass := Framelist.Frames[4].Frame;

    if Assigned(FAuthData) then
      FAuthData(Self, AUser, AuthVal)
        else AuthVal := True;

    AUser.Authed := AuthVal;

    if AuthVal then begin
      SendMessage(AUser.Socket, CMD_LOGIN_OK, 'Logged on');

      SendUserlist(AUser, ucsLogin);

      if Assigned(FConnectionsStatus) then
        FConnectionsStatus(Self, FClients.Count, FUsers.Count);
      end
        else begin
        SendMessage(AUser.Socket, CMD_LOGIN_FAIL, 'Login Disabled');
        AUser.Socket.Close; // Verbindung trennen
        end;
    end
      else begin
      SendMessage(AClient.Socket, CMD_USER_IN_USE, 'User allready connected'); // Benutzer bereits verbunden
      AClient.Socket.Close; // Verbindung trennen
      end;
end;

procedure TDC_SPA.FSocketClientDisconnect(Sender: TObject;
  Socket: TCustomWinSocket);
  var
    Index: Integer;
    TmpUser: TUser;
    SID: Integer;
begin

  if FUsers.FindSocket(Socket, Index) then begin
    TmpUser := FUsers.User[Index];
    SID := TmpUser.SID;
    TmpUser.Free;
    end;

  if FClients.FindSocket(Socket, Index) then begin
    FClients.Client[Index].SID := 0; // Nutze ich nun zur Prüfung ob der Client noch vorhanden ist, damit gehts ohne Fehler
    FClients.Client[Index].Free; // <-- Hier wird das Item in dem Fall der Inhalt aus AClient freigegeben
    end;

  if Assigned(FConnectionsStatus) then
    FConnectionsStatus(Self, FClients.Count, FUsers.Count);

  // Clients über die veränderung benachrichtigen
end;

Klappt das ohne Probleme wenn ich das Item von FClients aus nil setze?

[Muetze1]

Einfach mal meine Änderungen und Kommentare beachten...

Delphi-Quellcode:

procedure TDC_SPA.FSocketClientRead(Sender: TObject; Socket: TCustomWinSocket);
  var
    Index: Integer;
    AClient: TClient;
    DCFrame: String;
begin
  if FClients.FindSocket(Socket, Index) then begin
    AClient := FClients.Client[Index];

    AClient.Buffer := AClient.Buffer + Socket.ReceiveText;

    AClient.Buffer := StringReplace(AClient.Buffer, #$d#$a, LineFeed, [rfReplaceAll]);


    if AClient.Buffer > '' then begin
      while (Pos(EOCommand, AClient.Buffer) > 0) do begin
        Index := Pos(EOCommand, AClient.Buffer);
        DCFrame := Copy(AClient.Buffer, 1, Index + 4); // +4 ersetzten durch Length(EOCommand)
        AClient.Buffer := Copy(AClient.Buffer, Index + 5, Length(AClient.Buffer)); // +5 ersetzen

        Parse_DCFrame(AClient, DCFrame);

        if not Assigned(AClient) then
          Exit;  [b]// break würde reichen...[/b]

        if AClient.Buffer = '' then
          Exit;  [b]// break würde reichen...[/b]

        if AClient.SID = 0 then
          Exit;  [b]// break würde reichen...[/b]
        end;

      end;
    end;

end;

procedure TDC_SPA.Parse_DCFrame(AClient: TClient; AFrame: String);
  var
    Framelist: TFramelist;
    Command: Integer;
    AUser: TUser;
    Index: Integer;
begin
  if Assigned(FOnRawData) then
    FOnRawData(Self, rsIn, AClient.SID, AFrame);

  [...]

  case Command of // Kommandoframe auswerten
    CMD_AUTH: HandleLogin(AClient, Framelist); // Anmelden
    CMD_PONG: HandlePong(AClient, Framelist); // Pong
    CMD_SEND_USER: HandleUserMessage(AClient, FrameList); // Nachricht an einen Benutzer

    // Alle Commands > 100 werden extern gehandled;
    else begin
    // Sicherstellen das das Command über 100 liegt
      if Command < 100 then
        Exit;  [b]// was ist mit der Framelist? Speicherleck?[/b]

      // Nur wenn der Socket einem Benutzer zugeordnet werden kann fortfahren
      if Users.FindSocket(AClient.Socket, Index) then
        AUser := Users.User[Index]
          else Exit;  [b]// was ist mit der Framelist? Speicherleck?[/b]

      // Übergabe an Extern
      if Assigned(FOnCommand) then
        FOnCommand(Self, AUser, Command, Framelist);
      end;
  end;

  Framelist.Free;
end;

procedure TDC_SPA.HandleLogin(AClient: TClient; FrameList: TFrameList);
  var
    Index: Integer;
    AUser: TUser;
    AuthVal: Boolean;
begin
  if Framelist.Count <> 6 then // Anzahl der Frames MUSS 6 sein
    begin
    SendMessage(AClient.Socket, CMD_LOGIN_FAIL, 'Login Error. Not enough parameters'); // Fehlermeldung falls ungültig
    AClient.Socket.Close; // Socket freigeben
    Exit;
    end;

  if Not Users.FindUser( FrameList.Frames[1].Frame, Index ) then begin
    AUser := FUsers.AddConnection(AClient.Socket);  
    AUser.SID := AClient.SID;
    AUser.LoginTime := Now;
    AUser.LPStamp := TimeStamp;
    AUser.Address := FrameList.Frames[3].Frame;
    AUser.User := FrameList.Frames[1].Frame;
    AUser.Name := FrameList.Frames[2].Frame;
    AUser.Pass := Framelist.Frames[4].Frame;

    if Assigned(FAuthData) then
      FAuthData(Self, AUser, AuthVal)
        else AuthVal := True;

    AUser.Authed := AuthVal;

    if AuthVal then begin
      SendMessage(AUser.Socket, CMD_LOGIN_OK, 'Logged on');

      SendUserlist(AUser, ucsLogin);

      if Assigned(FConnectionsStatus) then
        FConnectionsStatus(Self, FClients.Count, FUsers.Count);
      end
        else begin
        SendMessage(AUser.Socket, CMD_LOGIN_FAIL, 'Login Disabled');
        AUser.Socket.Close; // Verbindung trennen
        end;
    end
      else begin
      SendMessage(AClient.Socket, CMD_USER_IN_USE, 'User [b]already[/b] connected'); // Benutzer bereits verbunden
      AClient.Socket.Close; // Verbindung trennen
      end;
end;

procedure TDC_SPA.FSocketClientDisconnect(Sender: TObject;
  Socket: TCustomWinSocket);
  var
    Index: Integer;
    TmpUser: TUser;
    SID: Integer;
begin

  if FUsers.FindSocket(Socket, Index) then begin
    TmpUser := FUsers.User[Index];
    SID := TmpUser.SID;
    TmpUser.Free;
    end;

  if FClients.FindSocket(Socket, Index) then begin
    FClients.Client[Index].SID := 0; // Nutze ich nun zur Prüfung ob der Client noch vorhanden ist, damit gehts ohne Fehler
    FClients.Client[Index].Free; // <-- Hier wird das Item in dem Fall der Inhalt aus AClient freigegeben
[/b]// Client ist TCollectionItem? Übernimmt die Collection die Einträge und gibt sie somit frei?
// Grundsätzlich solltest du hier aber nil dem ehemaligen Eintrag zuweisen nach dem Free. Damit müsstest
// du aber auch entsprechend darauf achten, dass du beim Zugriff auf die Collection NIL Einträge bekommen
// kannst. 
    end;

  if Assigned(FConnectionsStatus) then
    FConnectionsStatus(Self, FClients.Count, FUsers.Count);

  // Clients über die veränderung benachrichtigen
end;

Ok, alles mit dem Bold-Tag [b] beachten...

[Cyberaxx]

Das Speicherleck habe ich übersehen, das ist wirklich schlimm, das wird direkt behoben, danke :)

Das allererste mit den Exits war nut zum testen, hatte anfangs Break aber da übergang er es, mit Exit aber ebenfalls, darum bin ich ja drauf gekommen das eben noch Reste des AClient da sein müssen.
Die Procedure ist schon komplett umgeändert. Habe vor dem Free einfach die SID auf 0 gesetzt und darauf dann geprüft, ob das nun die sauberste Lösung ist glaube ich nicht aber immerhin konnte ich damit erstmal weiter arbeiten ohne eine Exception.

Ja die Collection übernimmt im Hintergrund alles. Das mit dem FreeandNil werde ich mal ausprobieren, sofern es auf Anhieb nicht zu viele Änderungen sind.

Was den gesamten Source angeht, so würde ich den wenn er mal fertig wird so oder so von dritten prüfen lassen, habe sicherlich noch überall kleine Mängel und Lecks drin, ganz zu schweigen von Optimierungen.

[Muetze1]

Zitat:

Zitat von Cyberaxx

Das Speicherleck habe ich übersehen, das ist wirklich schlimm, das wird direkt behoben, danke :)

Da hilft auch try/finally, weil trotz Exit würde der finally Block durchlaufen und somit wäre kein Leck entstanden.

Zitat:

Zitat von Cyberaxx

Die Procedure ist schon komplett umgeändert. Habe vor dem Free einfach die SID auf 0 gesetzt und darauf dann geprüft, ob das nun die sauberste Lösung ist glaube ich nicht aber immerhin konnte ich damit erstmal weiter arbeiten ohne eine Exception.

Oh, wow, dann empfehle ich dringends nochmal einen Lottoschein abzugeben. Du spielst hier auch Lotto, schliesslich gibst du die Instanz in der Liste einfach frei und später greifst du auf die nicht mehr existierende Instanz zu und vergleichst den SID Wert der Instanz. Da die TCollection immernoch die Adresse der alten Instanz hat aber an der Stelle keine Instanz mehr liegt, ist es wirklich ein sehr guter Zufall, dass an der ehemaligen Stelle von SID nun immernoch eine 0 steht. Damit: Lottoschein noch schnell abgeben - morgen ist Ziehung!

Wenn du die SID später vergleichen willst, dann muss die Instanz am Leben bleiben, welche die SID hält.

Zitat:

Zitat von Cyberaxx

Das mit dem FreeandNil werde ich mal ausprobieren, sofern es auf Anhieb nicht zu viele Änderungen sind.

Keine Chance, da Items[] eine Property von TCollection ist und da dahinter Getter und Setter stehen können und somit keine direkte Variable vorliegt, kann FreeAndNil() nicht angewendet werden. Du musst explizit nil zuweisen.

Zitat:

Zitat von Cyberaxx

Was den gesamten Source angeht, so würde ich den wenn er mal fertig wird so oder so von dritten prüfen lassen, habe sicherlich noch überall kleine Mängel und Lecks drin, ganz zu schweigen von Optimierungen.

Nicht meine Baustelle... :wink:

[Cyberaxx]

Zitat:

Zitat von Muetze1

Da hilft auch try/finally, weil trotz Exit würde der finally Block durchlaufen und somit wäre kein Leck entstanden.

Ja jetzt wo du es sagst stimmt es schon hab ich halt verpatzt, bzw nicht weiter drüber nach gedacht, da dieser Teil noch nicht wirklich implementiert ist.

Lotto wäre ne Idee :D

Was würde passieren wenn ich es vorher nochmal als eine Lokale Variable setze?

Delphi-Quellcode:

var
  AClient: TClient;
begin
  if FClients.FindSocket(Socket, Index) then 
    AClient := FClients.Client[Index];

  FreeandNil(AClient);
end;

[Edit]Ok vergiss den Teil, dann gibt es eine Exception in der Collection nehme ich an...[/Edit]

[Muetze1]

Es wäre komplett kein Unterschied. Delphi kennt keine auto-generated Copy Constructoren und somit hast du immer nur Zeiger auf die Objekte an sich. Somit ist es egal wie du den Aufruf des Destructors ausführst, es wird immer diese eine Instanz freigegeben. Die TCollection wie auch die lokale Variable oder AClient beinhalten immer nur die Adresse (sie ist intern ein Pointer) wo die Objektinstanz im Speicher liegt. Alle 3 zeigen auf einen Speicherbereich und wenn du diesen freigibst zeigen immernoch alle 3 Elemente auf diesen Speicherbereich (wo nichts mehr ist) und Zugriffe auf diese Elemente sind somit böse (eigentlich sollte man sofort was auf die Finger bekommen). Damit letzteres passiert, müssten die Adressen zurückgesetzt werden. Am besten eignet sich die Adresse 0 (nil), weil beim Zugriff auf solche Adressen werden vom Prozessor (durch die LDT/GDT) nicht erlaubt und er erzeugt eine entsprechende Ausnahme (endet bei uns in einer Exception) und wir bekommen eine entsprechende Meldung. Bei der ganzen Erklärung merkt man schon, dass es schwierig wird, alle Variablen welche auf ein und die gleiche Instanz zeigt, wieder ordentlich auf nil zu setzen, damit man mitbekommt ob die Instanz noch existiert. Zur Zeit hat noch ein anderer Threadersteller mit genau diesem Phänomen ein Problem.

Also: Welches Element du nun nimmst um es freizugeben: es ist immer nur eine Instanz da und die gibst du ja frei.

[Kedariodakon]

Ohne jetzt den ganzen Source gelesen zu haben, kann ich bei solchen Sachen einem nur das arbeiten mit Interfaces ans Herz legen...

Bye Christian

[Cyberaxx]

Hab nie wirklich etwas mit Interfaces gemacht und dein Satz gibt mir keinen wirklichen Ansatz warum ich dies tun sollte.
wäre nett wenn Du mit etwas mehr Informationen kommen würdest, warum ich Interfaces nutzen sollte.

Das einzige was mit in dem Projekt Sorgen bereitete war die Funktion des getrennten Benutzers.