Critical section in multithreading for ip scanner

Question

Hello i have doubt about critical section. I have two threads scannerThread and scannerchild thread.Code for Scannerthread is mentioned below.

procedure ScannerThread.Execute;
var
  I: Integer;
  ScannerCh: Array of ScannerChild;
  IpList: TStringlist;
  IPCount: Integer;
begin
  IpList:=TStringList.Create;
  IF GetNumberOfIpsInRange(Ip_From, Ip_To, IpList) then // Function call that returns iplist if TRUE
  begin
    Try
      if Assigned(LvHosts) then // Clear
        LvHosts.Clear;
      IPCount := IpList.Count;
      SetLength(ScannerCh, IPCount);
        I := 0;
        repeat
          while GetTThreadsCount(GetCurrentProcessId) > tcount do // Tcount is threads to create which is given by user     
            Sleep(10);
          ScannerCh[I]:=ScannerChild.Create(True, IpList[i]);
          ScannerCh[I].FreeOnTerminate := True;
          ScannerCh[I].LvHostname := LvHosts;
          ScannerCh[I].Resume;
          Inc(I);
        until I = IPCount;
      if Assigned(IpList) Then
        FreeAndNil(IpList);
    except
      ShowMessage('Operation Failed');
      If Assigned(IpList) Then
         FreeAndNil(IpList);
    end;
  end else
    ShowMessage('Invalid Range');
  repeat
   Sleep(100);
  Until GetTThreadsCount(GetCurrentProcessId) = 2;

end;

Code for Scannerchild is mentioned below

procedure ScannerChild.AddToList;
begin
  ListItems1 := LVHostName.Items.Add;
  ListItems1.Caption := IPToScan;
  ListItems1.SubItems.Add(IPAddrToName(IPToScan));
end;

procedure ScannerChild.AddToList1;
begin
  ListItems1:=LVHostName.Items.Add;
  ListItems1.Caption := IPToScan;
  ListItems1.SubItems.Add('No host found');
end;

procedure ScannerChild.Execute;
Var
  ListItems1 : TListItem;
  Hostname   : String;
begin       
    Hostname := IPAddrToName(IPToScan);
    if Hostname <> EmptyStr then
    begin
      Synchronize(AddToList);
    end else
      synchronize(AddToList1);
end;

Here I am getting output but it is not serialised. Like thread which getting created first is not getting displayed first. if I put ip range 192.168.0.1 to 192.168.0.10 then I should get serialised output in listview like

 192.168.0.1     hostname
 192.168.0.2     hostname
 192.168.0.3     hostname
 .
 .
 192.168.0.10    hostname

But i am not getting it.my output is coming like

    192.168.0.1    hostname
    192.168.0.2    hostname
    192.168.0.4    hostname 
    192.168.0.6    hostname
    192.168.0.3    hostname

So is it because I am not using critical section? Nnd if yes, where should I acquire and leave critical section so that thread created first can enter critical section first and next thread next like this.

Answer 1

Threads execute in parallel, independently of each other. Unless you impose some synchronisation or serialisation between the threads, the order in which they complete their tasks is unpredictable.

I suggest that you don't attempt to update the UI in a particular order because doing so will either hinder the performance of your scan, or make your code much more complex.

I do have a number of comments:

1. Separate the scanning from the UI. Such a design is cleaner and allows you to concentrate on each aspect independently. The way to make this happen is to store the results of the scan in a non-visual structure and then arrange that the GUI presents a view of this structure.
2. Don't hold references of free on terminate threads once you've started the thread because the reference can go stale any time.
3. Don't create threads to handle a single address. Use a thread pool.
4. Don't sleep.
5. Your code leaks IpList. Learn how to use finally.