Socket limit for Select on Linux (memory corruption)

[MaxRusov]

TIdSocketListVCLPosix.Add checks Count < FD_SETSIZE. It is incorrect in Posix. Need to check: AHandle < FD_SETSIZE.

_FD_SET causes memory corruption if AHandle > 1024.

It is advisable to replace select to poll in Linux

[rlebeau]

Is this working now?

[MaxRusov]

This is not enough for my purposes (WEB Server). I have a lot more than 1024 connections.

In my version, I use pool, it seems to work. But I'm handling a special case - only one socket at a time (Count = 1)

[rlebeau]

This is not enough for my purposes (WEB Server). I have a lot more than 1024 connections.

Per machine? Then how are you not running into file descriptor limitations outside of select()?

In any case, internally Indy uses only 1-2 sockets per TIdSocketList... instance. But I get your point. Indy will have to be updated in a future version to use poll() instead.

[dbcto]

Does the problem also exist in the Indy version that comes with Delphi 11? We also use Indy as a web server under Linux.

[rlebeau]

Does the problem also exist in the Indy version that comes with Delphi 11?

Yes, it does. And Embarcadero is already beta testing the next RAD Studio release, so a fix for this issue will likely not make it into that release, either. Maybe the release after that...

[MaxRusov]

Per machine? Then how are you not running into file descriptor limitations outside of select()?

Per process. Limit for file descriptors can be increased by 'ulimit'

Note. Each outgoing connection also occupies one handle. If process made 1024 outgoing connection - first incoming connection got Handle=1025 and can't be selected. OOPS. My server made some outgoing connection for each incoming, so limit much less then 1024 'users'

[dbcto]

Does the problem also exist in the Indy version that comes with Delphi 11?

Yes, it does. And Embarcadero is already beta testing the next RAD Studio release, so a fix for this issue will likely not make it into that release, either. Maybe the release after that...

That's good news. However, a new release of Embarcadero may take quite a while before it is released. Is there a relatively simple way or workaround to patch this in the current version?

[rlebeau]

Sorry for the delay in responding.

Is there a relatively simple way or workaround to patch this in the current version?

Actually, yes, I believe so. You should be able to derive a new class from TIdSocketList (nothing in TIdSocketListVCLPosix is worth inheriting) and override all of its virtual methods to operate with poll(), for example (feel free to adjust this code as needed):

type
  TMySocketList = class(TIdSocketList)
  protected
    FSockets: TList<TIdStackSocketHandle>;
    //
    class function FDPoll(ASet: TList<pollfd>; const ATimeout: Integer): Boolean;
    function GetItem(AIndex: Integer): TIdStackSocketHandle; override;
  public
    constructor Create; override;
    destructor Destroy; override;
    procedure Add(AHandle: TIdStackSocketHandle); override;
    procedure Remove(AHandle: TIdStackSocketHandle); override;
    function Count: Integer; override;
    procedure Clear; override;
    function Clone: TIdSocketList; override;
    function ContainsSocket(AHandle: TIdStackSocketHandle): Boolean; override;
    procedure GetPollList(VSet: TList<pollfd>; const AEvents: Int16);
    procedure SetPollList(VSet: TList<pollfd>; const AEvents: Int16);
    class function Select(AReadList: TIdSocketList; AWriteList: TIdSocketList;
      AExceptList: TIdSocketList; const ATimeout: Integer = IdTimeoutInfinite): Boolean; override;
    function SelectRead(const ATimeout: Integer = IdTimeoutInfinite): Boolean; override;
    function SelectReadList(var VSocketList: TIdSocketList;
      const ATimeout: Integer = IdTimeoutInfinite): Boolean; override;
  end;

constructor TMySocketList.Create;
begin
  inherited;
  FSockets := TList<TIdStackSocketHandle>.Create;
end;

destructor TMySocketList.Destroy;
begin
  FSockets.Free;
  inherited;
end;

procedure TMySocketList.Add(AHandle: TIdStackSocketHandle);
begin
  Lock;
  try
    if not FSockets.Contains(AHandle) then begin
      FSockets.Add(AHandle);
    end;
  finally
    Unlock;
  end;
end;

procedure TMySocketList.Clear;
begin
  Lock;
  try
    FSockets.Clear;
  finally
    Unlock;
  end;
end;

function TMySocketList.Clone: TIdSocketList;
begin
  Result := TMySocketList.Create;
  try
    Lock;
    try
      TMySocketList(Result).FSockets.Assign(FSockets);
    finally
      Unlock;
    end;
  except
    FreeAndNil(Result);
    raise;
  end;
end;

function TMySocketList.ContainsSocket(
  AHandle: TIdStackSocketHandle): Boolean;
begin
  Lock;
  try
    Result := FSockets.Contains(AHandle);
  finally
    Unlock;
  end;
end;

function TMySocketList.Count: Integer;
begin
  Lock;
  try
    Result := FSockets.Count;
  finally
    Unlock;
  end;
end;

class function TMySocketList.FDPoll(ASet: TList<pollfd>;
  const ATimeout: Integer): Boolean;
begin
  Result := poll(ppoolfd(ASet.List), ASet.Count, ATimeout) > 0;
end;

procedure TMySocketList.GetPollList(var VSet: TList<pollfd>;
  const AEvents: Int16);
var
  LPollFD: pollfd;
  I: Integer;
begin
  Lock;
  try
    for I := 0 to FSockets.Count-1 do begin
      LPollFD.fd := FSockets[i];
      LPollFD.events := AEvents;
      LPollFD.revents := 0;
      VSet.Add(LPollFD);
    end;
  finally
    Unlock;
  end;
end;

function TMySocketList.GetItem(AIndex: Integer): TIdStackSocketHandle;
begin
  Lock;
  try
    if (AIndex >= 0) and (AIndex < FSockets.Count) then begin
      Result := FSockets[AIndex];
    end else begin
      Result := -1;
    end;
  finally
    Unlock;
  end;
end;

procedure TMySocketList.Remove(AHandle: TIdStackSocketHandle);
begin
  Lock;
  try
    FSockets.Remove(AHandle);
  finally
    Unlock;
  end;
end;

class function TMySocketList.Select(AReadList, AWriteList,
  AExceptList: TIdSocketList; const ATimeout: Integer): Boolean;
var
  LPollList: TList<pollfd>;

  procedure SetCount(AList: TIdSocketList);
  begin
    if AList <> nil then begin
      Result := AList.Count;
    end else begin
      Result := 0;
    end;
  end;

  procedure ReadSet(AList: TIdSocketList; const AEvents: Shortint);
  begin
    if AList <> nil then begin
      TMySocketList(AList).GetPollList(LPollList, AEvents);
    end;
  end;

begin
  LPollList := TList<pollfd>.Create;
  try
    LPollList.Capacity := SetCount(AReadList) + SetCount(AWriteList) + SetCount(AExceptList);
    ReadSet(AReadList, POLLIN or POLLRDHUP);
    ReadSet(AWriteList, POLLOUT);
    ReadSet(AExceptList, POLLPRI);
    //
    Result := FDPoll(LPollList, ATimeout);
    //
    if AReadList <> nil then begin
      TMySocketList(AReadList).SetPollList(LPollList, POLLIN or POLLHUP);
    end;
    if AWriteList <> nil then begin
      TMySocketList(AWriteList).SetPollList(LPollList, POLLOUT);
    end;
    if AExceptList <> nil then begin
      TMySocketList(AExceptList).SetPollList(LPollList, POLLPRI);
    end;
  finally
    LPollList.Free;
  end;
end;

function TMySocketList.SelectRead(const ATimeout: Integer): Boolean;
var
  LPollList: TList<pollfd>;
begin
  LPollList := TList<pollfd>.Create;
  try
    Lock;
    try
      LPollList.Capacity := FSockets.Count;
      GetPollList(LPollList, POLLIN or POLLRDHUP);
    finally
      Unlock;
    end;
    Result := FDPoll(LPollList, ATimeout);
  finally
    LPollList.Free;
  end;
end;

function TMySocketList.SelectReadList(var VSocketList: TIdSocketList;
  const ATimeout: Integer): Boolean;
var
  LPollList: TList<pollfd>;
begin
  LPollList := TList<pollfd>.Create;
  try
    Lock;
    try
      LPollList.Capacity := FSockets.Count;
      GetPollList(LPollList, POLLIN or POLLRDHUP);
    finally
      Unlock;
    end;
    Result := FDPoll(LPollList, ATimeout);
    if Result then begin
      if VSocketList = nil then begin
        VSocketList := TIdSocketList.CreateSocketList;
      end;
      TMySocketList(VSocketList).SetPollList(LPollList, 0);
    end;
  finally
    LPollList.Free;
  end;
end;

procedure TMySocketList.SetPollList(VSet: TList<pollfd>;
  const AEvents: Int16;
var
  LPollFD: pollfd;
  I: Integer;
begin
  Lock;
  try
    FSockets.Clear;
    for I := 0 to VSet.Count-1 do begin
      LPollFD := VSet[I];
      if (AEvents = 0) or ((LPollFD.revents and AEvents) <> 0) then begin
        FSockets.Add(LPollFD.fd);
      end;
    end;
  finally
    Unlock;
  end;
end;

And then, at runtime you can assign your new class type to Indy's global GSocketListClass variable in the IdStack unit:

IdStack.GSocketListClass := TMySocketList;

The default platform-specific TIdSocketListXXX classes are all dependent on their platform's fd_set type, and they are unfortunately declared in the interface section of their respective IdStackXXX units, so applying this fix in the main code will require interface-breaking changes. Since all of the classes logic are private, the classes should have all been declared in their respective unit's implementation section rather than the interface section, then this would not have been an issue to patch. I'll have to fix that in a future version.

[dbcto]

Thank you very much! We will try that and let you know if it worked.

[msccto]

@rlebeau I tried to implement the workaround you suggest using your code above. However I run into several problems and can not compile it. For example, FSockets is of type System.Generics.Collections.TList<>: FSockets: TList<TIdStackSocketHandle>; but there is no Assign method available causing a compile error in TMySocketList(Result).FSockets.Assign(FSockets);

Another problem is that I don't have poll function, causing an error in Result := poll(ppoolfd(ASet.List), ASet.Count, ATimeout) > 0;

Am I missing something? Appreciate any help.

[rlebeau]

However I run into several problems and can not compile it.

I wrote that example from scratch off the top of my head, I didn't actually test it. I did say to "feel free to adjust this code as needed", which would include tweaking it to make it compile :-)

there is no Assign method available

The non-generic TList has an Assign() method, so I assumed the generic TList did as well. Apparently not. In which case, use the Clear() and AddRange() methods instead, eg:

with TMySocketList(Result).FSockets do
begin
  Clear;
  AddRange(Self.FSockets);
end;

Another problem is that I don't have poll function

Obviously, you will need to add the appropriate declaring unit in your uses clause (offhand, I don't know which unit that is), or else declare the function in your own code.

[rlebeau]

How is this looking? Were you able to get it working?

[msccto]

We were looking for another solution so far, however we try to get it working using your approach in the next days. We'll keep you posted.

[msccto]

I finally was able to get it working using your code and it works good so far. My test application using TIdHTTPServer with original implementation crashed on Linux when there were more than 1024 clients trying to connect - with the new implementation it worked very good with more than 7000 clients.

I also created a PR #473