Access Violation when i free idUDPClient

[ChrisChuah 0]

Hi

I am trying to create a thread that will send out UDP message and when it receive the UDP message from the server, it will send a Reply as event back to the caller.

I create the UDPClient in the Execute function and of course i would need to free it when the thread ends.

But why do i get access violation when i free it?

The call stack is

--IdSocketHandle.CloseSocket.

--IdSocketHandle.Destroy

-- System.TObject.free;

--IdUDPBase.CloseBinding

--IdUDPBase.Destroy

--IdUDPClient.Destroy

 -- System.TObject.Free

--FUDP.Free;

 

please help to guide me what is the best way to send out UDP message and wait for reply?

Should i use UDPServer rather than UDPClient?

 

=== Code ===

procedure ilUDPClientThd.Execute;
var
  l_port : UInt32;
  l_memStream : TMemoryStream;
  l_len : integer;
  l_buf : TIdBytes;
  l_ilUDPData : Tt_BootReqMsg;
  l_ip : string;
  l_ipBytes : array[0..15] of byte;
  FUDP: TIdUDPClient;
begin
  { Place thread code here }
  l_ip := getOwnIP;
  if l_ip = '' then begin
    DoOnLog('Cannot find own IP address');
    exit;
  end;
  FUDP := TIdUDPClient.Create(nil);
  FUDP.Host := FBroadcastIP;
  FUDP.Port := FBroadcastPort;
//  FUDP.Binding.Port := FBroadcastPort;
  try
    l_port := FBroadcastPort;
    l_memStream := TMemoryStream.create;

    l_buf := ToBytes('SOREQ', IndyTextEncoding_ASCII);
    l_memStream.Write(l_buf[0], 6);

    l_port := GStack.HostToNetwork(l_port);
    l_memStream.write(l_port, 4);

    getArrayBytes(l_ip, l_ipBytes);
    l_memStream.Write(l_ipBytes, SizeOf(l_ipBytes));

    l_len := l_memStream.size;
    setLength(l_buf, l_len);
    l_memStream.position := 0;
    l_memStream.Read(l_buf[0], l_len );
    l_memStream.Clear;
    try
      FUDP.ReceiveTimeout := 3000;
      FUDP.BroadcastEnabled := true;
//      FUDP.Active := true;
      FUDP.SendBuffer(l_buf);
//      FUDP.SendBuffer(l_buf[0], l_len);
      FMsg := 'Send To Address: ' + FUDP.Host + ' Port: ' + Inttostr(FUDP.Port) + ' using IP: ' + l_ip;
      Synchronize(WriteToLog);
//      l_len := FUDP.ReceiveBuffer(l_ilUDPData, sizeof(l_ilUDPData), FUDP.ReceiveTimeout);
      setLength(l_buf, sizeof(l_ilUDPData));
      l_len := FUDP.ReceiveBuffer(l_buf);
      if l_len <= 0 then begin
        FMsg:='No Data received in UDP';
        FRepliedClientIP := '';
        FRepliedClientPort := 0;
        Synchronize(ReplyToClient);
        Synchronize(WriteToLog);
      end
      else begin
        l_ilUDPData.Port := GStack.NetworkToHost(l_ilUDPData.Port);
        FRepliedClientIP := getStringFromBytes(l_ilUDPData.hid);
        FRepliedClientPort := l_ilUDPData.port;
        Synchronize(ReplyToClient);
      end;
    except
      on E:Exception do begin
        FMsg := 'Error in Recv Data in UDP: ' + e.message;
        FRepliedClientIP := '';
        FRepliedClientPort := 0;
        Synchronize(ReplyToClient);
        Synchronize(WriteToLog);
      end;
    end;
    l_memStream.Free;
    FUDP.Binding.Destroy;
  except
    on E:exception do begin
      FMsg := 'Exception in SoFind: ' + e.Message;
      FRepliedClientIP := '';
      FRepliedClientPort := 0;
      Synchronize(ReplyToClient);
      Synchronize(WriteToLog);
    end;
  end;
  FUDP.Disconnect;
  FUDP.Free; <<==== Access violation when i tried to free UDP Client 
end;

[Remy Lebeau 1436]

2 hours ago, ChrisChuah said:

FUDP.Binding.Destroy;

Why are you doing this? This is the root of your crash. You don't own the Binding object, so you have no business destroying it like this.  Let TIdUDPClient manage the Binding for you.  It will be destroyed automatically when the TIdUDPClient itself is destroyed.

 

The stack trace you have provided clearly shows TIdUDPClient's destructor is trying to access the Binding after you have manually destroyed it, hence the crash.

 

If you really want to destroy the Binding manually, you need to instead set the TIdUDPClient.Active property to False, which will internally close and destroy the Binding if it is currently active:

//FUDP.Binding.Destroy; 
FUDP.Active := False;

Quote

Should i use UDPServer rather than UDPClient?

No.  That would just complicate your code further, since TIdUDPServer is asynchronous, whereas TIdUDPClient is synchronous.

 

That being said, I see some other issues with your code...

Quote

FUDP := TIdUDPClient.Create(nil);

...
FUDP.Free;

You should wrap the TIdUDPClient in a try/finally block to ensure that it gets Free()'d in case an exception is raised:

FUDP := TIdUDPClient.Create(nil);
try
  ...
finally
  FUDP.Free;
end;

Same with the TMemoryStream.

Quote

l_buf := ToBytes('SOREQ', IndyTextEncoding_ASCII);
l_memStream.Write(l_buf[0], 6);

This is a buffer overflow, as l_buf has 5 bytes in it but you are asking Write() to read 6 bytes from it. l_buf is not null-terminated, so if you need it to be then you have to either add an explicit terminator to the string:

l_buf := ToBytes('SOREQ'#0, IndyTextEncoding_ASCII);
l_memStream.Write(l_buf[0], Length(l_buf));

Or, write the null-terminator separately:

const NullTerm: AnsiChar = #0;
...
l_buf := ToBytes('SOREQ', IndyTextEncoding_ASCII);
l_memStream.Write(l_buf[0], Length(l_buf));
l_memStream.Write(NullTerm, 1);

Alternatively, use Indy's own WriteStringToStream() function in the IdGlobal unit:

WriteStringToStream(l_memStream, 'SOREQ'#0, IndyTextEncoding_ASCII);

WriteStringToStream(l_memStream, 'SOREQ', IndyTextEncoding_ASCII);
WriteStringToStream(l_memStream, #0, IndyTextEncoding_ASCII);

In any case, you are converting the TMemoryStream data to a TIdBytes for transmission, so you might consider simply getting rid of the TMemoryStream altogether and use Indy's own TIdBytes-oriented functions instead (CopyTIdString(), CopyTIdUInt32(), CopyTIdByteArray(), etc), which are also in the IdGlobal unit:

SetLength(l_buf, 6 + SizeOf(l_port) + SizeOf(l_ipBytes));

CopyTIdString('SOREQ'#0, l_buf, 0, -1, IndyTextEncoding_ASCII);

l_port := GStack.HostToNetwork(l_port);
CopyTIdUInt32(l_port, l_buf, 6);

getArrayBytes(l_ip, l_ipBytes);
CopyTIdByteArray(l_ipBytes, 0, l_buf, 10, SizeOf(l_ipBytes));

Quote

l_len := FUDP.ReceiveBuffer(l_buf);
if l_len <= 0 then begin

You are not checking to make sure that l_len specifies that enough bytes are actually available to read from l_buf into l_ilUDPData. There is no guarantee that the received datagram is actually as large as your buffer.

Quote

FUDP.Disconnect;

You are not Connect()'ing the TIdUDPClient to a specific peer, so this line is useless and can be removed. Especially since the next statement is Free()'ing the TIdUDPClient anyway.

Edited May 26, 2022 by Remy Lebeau

[ChrisChuah 0]

Thanks Remy

The only problem that i have with strings in delphi is that I dont know if they are Windows widechar or UTF8 char or ansi value

 

Edit1.text := 'MET';

 

ShowMe(Edit1.text);

 

Function showMe(aValue : string);

var

  l_value : string;

begin

  l_value := aValue + 'A';

  UDPClient.Write(l_value);

end;

 

So when the edit1 value is pass into ShowMe function, what is the length of l_value? 

As my server only accepts ANSI type of characters, the data sent may be Windows WideChar.

Hence to be on the safe side, i would have to translate it to idBytes.

 

What is the general rule on how you would send data when the server only accepts ANSI characters.

 

Please advise

regards

chris

 

BTW, After 20 years of using Delphi, i still remembered you were helping me last time when i was using Delphi 5, 6 in one of the forum.

Now, its hard to find help for delphi.

 

 

[ChrisChuah 0]

Hi Remy

Would like to ask why the UDPClient did not receive any data.

Attached is the screen capture of the Wireshark.

The first pic shows the server received the message sent from the delphi client

The second pic shows the server sends out the SORE (response) message to the delphi client IP address.

However, the client reported that there is no data received

 

<27/5/2022 11:14:02 am> Send To Address: 192.168.84.255 Port: 55510 using IP: 192.168.84.129
<27/5/2022 11:14:05 am> IP:  Port: 0 replied
<27/5/2022 11:14:05 am> No Data received in UDP

 

Did i do something not right?

please advise

 

regards

chris

 

 

[ChrisChuah 0]

Hi

When i use the IdUDPClient inside the thread object, the idUDPClient does not get any messages back even though it can be seen from the Wireshark that the message was sent back.

However, if i place the IdUDPClient on a Form, i am able to get the message.

 

Something funny is that when i use the IdUDPClient on the Form to send out the message, 

the first response message header is SOREQ <== this header is request header

Checking on the Wireshark, the server returned SORE

I tried to initialise the l_buf to $0 before retrieving from idUDPClient.

However, its only the first message sent that will be always SOREQ and the rest will be SORE

And the IP address returned in the message should be 192.168.84.128 which indicate the server address itself.

seems like the first message is the same message that i sent out.

 

 

please advise

 

regards

chris

 

===== Code for IdUDPClient on Form =====

procedure TForm1.Button2Click(Sender: TObject);
var
  l_port : UInt32;
  l_memStream : TMemoryStream;
  l_len : integer;
  l_buf, l_buf1 : TIdBytes;
  l_ilUDPData : Tt_BootReqMsg;
  l_ip : string;
  l_ipBytes : array[0..15] of byte;
  FMsg : string;
  FRepliedClientIP : string;
  FRepliedClientPort : Uint32;
  l_index : integer;
begin
  l_ip := getOwnIP;
  if l_ip = '' then begin
    writeLog('Cannot find own IP address');
    exit;
  end;
  IdUDPClient1.Host := '192.168.84.255';
  IdUDPClient1.Port := 55510;
  IdUDPClient1.BoundPort  := 55510;
  try
//    FUDP.Active := true;
    l_memStream := TMemoryStream.create;

    l_buf := ToBytes('SOREQ'#0, IndyTextEncoding_ASCII);
    l_memStream.Write(l_buf[0], 6);

    l_port := 55510;
    l_port := GStack.HostToNetwork(l_port);
    l_memStream.write(l_port, 4);

    getArrayBytes(l_ip, l_ipBytes);
    l_memStream.Write(l_ipBytes, SizeOf(l_ipBytes));

    l_len := l_memStream.size;
    setLength(l_buf, l_len);
    l_memStream.position := 0;
    l_memStream.Read(l_buf[0], l_len );
    l_memStream.Clear;
    try
      IdUDPClient1.ReceiveTimeout := 3000;
      IdUDPClient1.BroadcastEnabled := true;
      IdUDPClient1.SendBuffer(l_buf);
      FMsg := 'Send To Address: ' + IdUDPClient1.Host + ' Port: ' + Inttostr(IdUDPClient1.Port) + ' using IP: ' + l_ip;
      WriteLog(FMsg);
      setLength(l_buf, sizeOf(l_ilUDPData));
      for l_index := 0 to length(l_buf) - 1 do. <== initialise the buffer but i still receive SOREQ only on 1st time.
        l_buf[l_index] := byte($0);
      l_len := IdUDPClient1.ReceiveBuffer(l_buf, 2000);
      if l_len <= 0 then begin
        FMsg:='No Data received in UDP';
        FRepliedClientIP := '';
        FRepliedClientPort := 0;
        WriteLog(FMsg);
      end
      else begin
        l_memStream.Clear;
        l_memStream.Position := 0;
        l_memStream.Write(l_buf[0], l_len);
        l_memStream.Position := 0;
        setlength(l_buf1, 6);
        l_memStream.Read(l_buf1[0], length(l_buf1));
        WriteLog('Op Code: ' + getStringFromBytes(l_buf1));
        l_memStream.Read(FRepliedClientPort, sizeof(FRepliedClientPort));
        WriteLog('Port value: ' + Inttostr(FRepliedClientPort));
        l_memStream.Read(l_ilUDPData.hid, sizeof(l_ilUDPData.hid));
        FRepliedClientIP := getStringFromBytes(l_ilUDPData.hid);
        FRepliedClientPort := GStack.NetworkToHost(FRepliedClientPort);
        WriteLog('Replied. IP: ' + FRepliedClientIP + ' Port: ' + Inttostr(FRepliedClientPort));
      end;
    except
      on E:Exception do begin
        FMsg := 'Error in Recv Data in UDP: ' + e.message;
        FRepliedClientIP := '';
        FRepliedClientPort := 0;
        WriteLog(FMsg);
      end;
    end;
    l_memStream.Free;
  except
    on E:exception do begin
      FMsg := 'Exception in SoFind: ' + e.Message;
      FRepliedClientIP := '';
      FRepliedClientPort := 0;
      WriteLog(FMsg);
    end;
  end;

end;
 

==== End =====

 

[Remy Lebeau 1436]

16 hours ago, ChrisChuah said:

The only problem that i have with strings in delphi is that I dont know if they are Windows widechar or UTF8 char or ansi value

Very simple:

 

pre-D2009:

Char is AnsiChar

String is AnsiString, using ANSI encoded AnsiChars

UTF8String is just an alias for AnsiString, it is not guaranteed to be UTF-8 unless it comes from UTF8Encode()

 

post-D2009:

Char is WideChar

String is UnicodeString, using UTF-16 encoded WideChars

UTF8String is a native string type, using UTF-8 encoded AnsiChars

Quote

Edit1.text := 'MET';

ShowMe(Edit1.text);

...

So when the edit1 value is pass into ShowMe function, what is the length of l_value?

In you example, the length will be 4 Chars (after adding the 'A').  How many bytes those Chars will take up depends on the byte encoding you tell the UDPClient to use.  In ANSI/UTF-8, ASCII characters are 1 byte each.  In UTF-16, they are 2 bytes each.  Things get more complex when you start dealing with non-ASCII characters.

Quote

As my server only accepts ANSI type of characters, the data sent may be Windows WideChar.

First off, TIdUDPClient does have a Write() method, it has Send() and SendBuffer() methods instead.

In any case, internally TIdUDPClient will convert the String to bytes, and then transmit the bytes.  The default encoding used to produce those bytes is US-ASCII, but you can specify a different encoding in the optional AByteEncoding parameter of Send/Buffer(), or by setting the global GIdDefaultTextEncoding variable in the IdGlobal unit.

Quote

Hence to be on the safe side, i would have to translate it to idBytes.

Well, you have to do that anyway, since you are including binary data in your request, not just text.

Quote

What is the general rule on how you would send data when the server only accepts ANSI characters.

Explicitly tell TIdUDPClient (or any other Indy component) to transmit strings in the desired ANSI encoding.  Don't just rely on defaults.

15 hours ago, ChrisChuah said:

Would like to ask why the UDPClient did not receive any data.

It is because your server did not send the response to the correct port on the client.  The client sent out the request from port 59114, so that was the port it could receive the response on, but the server sent the response to port 55510 instead.  So, the server ignored the source port of the request and sent the response to its own port instead.  If the server requires the client to use the same port that the server is using, you will have to set the TIdUDPClient.BoundPort to that port before sending requests (which you seem to have already figured out).  Otherwise, fix the server to send the response to the request's actual source port.

Quote

When i use the IdUDPClient inside the thread object, the idUDPClient does not get any messages back even though it can be seen from the Wireshark that the message was sent back.

However, if i place the IdUDPClient on a Form, i am able to get the message.

Indy does not behave differently in a worker thread vs in the main UI thread.  It is not dependent on message handling.

Quote

Something funny is that when i use the IdUDPClient on the Form to send out the message, 

the first response message header is SOREQ <== this header is request header

...

seems like the first message is the same message that i sent out.

Your client is sending the request to a subnet broadcast IP (192.168.84.255) instead of to the server's actual IP (192.168.84.128) (why?).  And since you are not setting the TIdUDPClient.BoundIP to bind the client to a specific network adapter, that explains why your client is receiving a duplicate copy of the broadcasted request.  So, you will have to either:

- send to the server's IP directly.

- make your client ignore messages that have its own local IP as the source IP.

- set the TIdUDPClient.BoundIP to the client's own local IP, then it won't receive broadcasts from itself.

Edited May 27, 2022 by Remy Lebeau

[ChrisChuah 0]

Thanks Remy.