Quick check on dynamic array deprecation and any resulting memory leaks

A little information on how this program works:

Main winsock component waits for new connections.
When a new connection arrives it takes the dynamic  array holding all the connections and sets the length to one larger.(I.E. SetLength(ServerClients,TotalConnections + 1)). In the new space allocolated it creates a new winsock object to take the connections.(Shown below).

Now when the connection is closed I FreeAndNil() that spot in the dynamic array. So you may have (con1,con2,nil,con4). What I have done(also shown below) is first take the nil out of the array by moving the next one in the array down. You would then have (con1, con2, con4, con4). I then reset all tags and information to reorder the array to (con1, con2, con3, con4). Now that con4 isn't necessary I set the length down one.

Now I thought I would have to put a FreeAndNil() for the con4 but that just gives me errors. It works without it but I wonder if I'm leaking memory this way.

Any suggestions on how this could be done differently? Or if it's fine as it is?
procedure TForm1.Winsock1ConnectionRequest(ASender: TObject;
  requestID: Integer);
var
  CurrentClient : TClientWinsock;
begin
  Inc(TotalClients);
  SetLength(ServerClients,TotalClients);
  ServerClients[TotalClients - 1] := TClientWinsock.Create(Winsock1);
  CurrentClient := ServerClients[TotalClients - 1];
  CurrentClient.LocalPort := LocalPort;
  CurrentClient.Tag := TotalClients - 1;
  CurrentClient.Ping := 0;
  CurrentClient.IdleTime := 0;
  CurrentClient.CurrentUser := '';
  CurrentClient.ActiveWindow := '';
  CurrentClient.OnDataArrival := WinsockDataArrival;
  CurrentClient.OnClose := WinsockClose;
  CurrentClient.OnError := WinsockError;
  CurrentClient.Close;
  CurrentClient.Accept(requestID);
end;
 
 
procedure TForm1.WinsockClose(Sender: TObject);
var
  I,J,Max : Integer;
  CurrentClient : TClientWinsock;
begin
  CurrentClient := (Sender as TClientWinsock);
  CurrentClient.Close;
  J := CurrentClient.Tag;
  ComboBox1.Items.Clear;
  FreeAndNil(ServerClients[J]);
  Dec(TotalClients);
  For I := J To High(ServerClients) Do
    begin
    If I = High(ServerClients) Then
      begin
      SetLength(ServerClients,TotalClients);
      Break;
    end;
    ServerClients[I] := ServerClients[I + 1];
    //FreeAndNil(ServerClients[I + 1]);
    ServerClients[I].Tag := ServerClients[I].Tag - 1;
  end;
  For I := Low(ServerClients) To High(ServerClients) Do
    begin
    ComboBox1.Items.AddObject(ServerClients[I].RemoteHostIP,TObject(ServerClients[I].Tag));
  end;
  Panel2.Caption := '';
  Panel3.Caption := '';
  Panel4.Caption := '';
  Panel5.Caption := '';
  Panel6.Caption := '';
end;

Open in new window

LVL 13
ThievingSixAsked:
Who is Participating?
 
Ustin19Connect With a Mentor Commented:
dinilud is on point: it is no memory leaks
(con1, con2, con3, con4) - is pointers to objects, when (con1, con2, con4, con4) reduces to (con1, con2, con4) pointer to con4 stores in element [2] and all right.
Error appears when you freeandnil con4 (!) at first step of loop and then use it's method ServerClients[I].Tag at next iteration :)
0
 
diniludConnect With a Mentor Commented:
i think , there is no memory leak.

you can check this with 'EurekaLog' or 'FastMM'
0
 
Geert GConnect With a Mentor Oracle dbaCommented:
why don't you use a TCollection with a descendant of TCollectionItem ?
0
 
Geert GOracle dbaCommented:
saves all the hassle with using a collection
no need to reorder
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.