Pinpointing the cause of a EInvalidPointer exception when destroying Indy components

Question

I'm trying to figure out how to resolve a race condition that arises when destroying Indy components. We are receiving sporadic EInvalidPointer exceptions during form destruction when we have an indy TIdHTTPServer component on the form.

Background

We have a simple form with a TIdHTTPServer on it, and a single OnCommandGet event:

procedure TForm2.httpCommandGet(AContext: TIdContext;
  ARequestInfo: TIdHTTPRequestInfo; AResponseInfo: TIdHTTPResponseInfo);
var
  Filename: string;

  function RespondFile: Boolean;
  var
    EnableTransferFile: Boolean;
    SourceFilename: string;
  begin
    SourceFilename := CSourcePath + Filename;
    if not FileExists(SourceFilename) then
      Exit(False);

    AResponseInfo.ContentType := http.MIMETable.GetFileMIMEType(SourceFilename);
    AResponseInfo.ContentLength := FileSizeByName(SourceFilename);
    AResponseInfo.WriteHeader;
    EnableTransferFile := not (AContext.Connection.IOHandler is TIdSSLIOHandlerSocketBase);
    AContext.Connection.IOHandler.WriteFile(SourceFilename, EnableTransferFile);
    Result := True;
  end;
begin
  Filename := ARequestInfo.Document;
  if Copy(Filename, 1, 1) = '/' then Delete(Filename, 1, 1);
  if Filename.Contains('/') or Filename.Contains('\') then
  begin
    // Block path attempts
    AResponseInfo.ResponseNo := 404;
    Exit;
  end;

  if not RespondFile then
    AResponseInfo.ResponseNo := 404;
end;

The http server is set Active=True at design time.

Running the program

The program serves a simple web page which repeatedly contacts the web server (via XmlHttpRequest) and downloads itself. This provides a trigger for the race condition.

FormDestroy and Active := False (a small diversion)

When I destroy the form, I know that I need to set Active:=False in the FormDestroy event, because otherwise the OnCommandGet event, which is called on an Indy thread, not on the main thread, can be called while the form is being destroyed, and form members and components are in an indeterminate state. We cannot test the csDestroying state of the form at entry of the callback, because this can be set at any time, even while our event is running.

If we don't reference any members of the form class within the callback, this race condition does not really cause a problem. In practice, however, we often need to access members of the form (with suitable locks), and so setting Active:=False in the FormDestroy event means we can control when the listener threads are torn down. All good so far.

procedure TForm2.FormDestroy(Sender: TObject);
begin
  http.Active := False;
end;

The crash

However, we still occasionally get an EInvalidPointer exception when destroying the form. This arises in TIdYarnOfThread.Destroy in a httpScheduler User thread:

:7775d8a8 KERNELBASE.RaiseException + 0x48
System.TObject.FreeInstance
System.ErrorAt(2,$407841)
System.Error(reInvalidPtr)
System.TObject.FreeInstance
System._ClassDestroy(???)
IdSchedulerOfThread.TIdYarnOfThread.Destroy
System.TObject.Free
IdThread.TIdThread.Cleanup
IdThread.TIdThread.Execute
System.Classes.ThreadProc($2DD5000)
System.ThreadWrapper($2E510C0)
:75c938f4 KERNEL32.BaseThreadInitThunk + 0x24
:77a65663 ; 
:77a6562e ;

The corresponding main thread stack is:

:77a76fec ntdll.NtDelayExecution + 0xc
:7775a4ef KERNELBASE.Sleep + 0xf
IdGlobal.IndySleep(???)
IdScheduler.TIdScheduler.TerminateAllYarns
IdCustomTCPServer.TIdCustomTCPServer.TerminateAllThreads
IdCustomTCPServer.TIdCustomTCPServer.Shutdown
IdCustomHTTPServer.TIdCustomHTTPServer.Shutdown
IdCustomTCPServer.TIdCustomTCPServer.SetActive(???)
idyracemain.TForm2.Timer1Timer($32FDCC0)
Vcl.ExtCtrls.TTimer.Timer
Vcl.ExtCtrls.TTimer.WndProc(???)
System.Classes.StdWndProc(10552024,275,1,0)
:775384e3 user32.SetManipulationInputTarget + 0x53
:77516c30 ; C:\WINDOWS\SysWOW64\user32.dll
:77516531 ; C:\WINDOWS\SysWOW64\user32.dll
:775162f0 user32.DispatchMessageW + 0x10
Vcl.Forms.TApplication.ProcessMessage(???)
:005c22b0 TApplication.ProcessMessage + $F8

Note: the stack above is from a stress test that forces this issue with a timer, hence the reference to TTimer.

An EInvalidPointer exception raised from System.TObject.FreeInstance is usually a double-free. I have been debugging this but am not yet familiar enough with TIdYarn to understand its complete lifecycle. However, the following procedure may be the cause:

procedure TIdSchedulerOfThread.TerminateYarn(AYarn: TIdYarn);
var
  LYarn: TIdYarnOfThread;
begin
  Assert(AYarn<>nil);
  LYarn := TIdYarnOfThread(AYarn);
  if (LYarn.Thread <> nil) and (not LYarn.Thread.Suspended) then begin
    // Is still running and will free itself
    LYarn.Thread.Stop;
    // Dont free the yarn. The thread frees it (IdThread.pas)
  end else
  begin
    // If suspended, was created but never started
    // ie waiting on connection accept

    // RLebeau: free the yarn here as well. This allows TIdSchedulerOfThreadPool
    // to put the suspended thread, if present, back in the pool.

    IdDisposeAndNil(LYarn);
  end;
end;

The problem here is this procedure runs in thread context of the owning component (often the main thread). After the procedure tests if the thread is suspended (i.e. not yet started), the thread may be started from another thread, causing the LYarn object to be freed twice - once by this function, and once by its owning thread.

But I may not have the right end of the stick. Is this exception a result of a mistake in the way I am using the TIdHttpServer component, and if so what am I doing wrong and how do I resolve it?

Update: MCVE

The following program forces the issue reasonably quickly on my machine using Delphi 10 Seattle (release version) with the included Indy components. Because this appears to be a thread race condition, YMMV on different hardware; you may find a single core VM helpful to repro the problem. You must run in a debugger to capture the EInvalidPointer exception, otherwise it is silently handled.

When testing, I have seen, very infrequently, an EAccessViolation, and a hang with the same main thread call stack as the EInvalidPointer. I suspect these may all have the same root cause.

Add a TIdHTTPServer, TTimer, TLabel and TWebBrowser to a new form, and attach the events TIdHTTPServer.OnCommandGet, TTimer.OnTimer and Form.OnCreate as below. Set TIdHTTPServer.Bindings[0]=127.0.0.1,9999, and KeepAlive=True. KeepAlive=True tends to trigger the problem more quickly on my machine but it does still happen with KeepAlive=False.

When you run the program in the Delphi debugger, you will normally receive an EInvalidPointer within a few minutes. You can often trigger it more rapidly by breakpointing on TIdThread.Cleanup and just continuing when the debugger breaks in.

unit idyracemain;

interface

uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, IdContext, IdCustomHTTPServer,
  IdBaseComponent, IdComponent, IdCustomTCPServer, IdHTTPServer, Vcl.ExtCtrls,
  Vcl.StdCtrls, Vcl.OleCtrls, SHDocVw;

type
  TForm2 = class(TForm)
    http: TIdHTTPServer; //Bindings[0]=127.0.0.1:9999, KeepAlive=True
    Timer1: TTimer;
    Label1: TLabel;
    WebBrowser1: TWebBrowser;
    procedure httpCommandGet(AContext: TIdContext;
      ARequestInfo: TIdHTTPRequestInfo; AResponseInfo: TIdHTTPResponseInfo);
    procedure Timer1Timer(Sender: TObject);
    procedure FormCreate(Sender: TObject);
  private
  public
  end;

var
  Form2: TForm2;

implementation

{$R *.dfm}

const doc =
  '<!DOCTYPE html>'+
  '<html lang="en"><head><title>Indy Stress</title></head><body>'+
  'Attempts: <span id="counter">0</span>'+
  '<script>'+
  '  var attempts = 0;'+
  '  window.setInterval(function() {  '+
  '    var x = new XMLHttpRequest();'+
  '    document.getElementById("counter").innerText = ++attempts;'+
  '    x.open("GET", "/");'+
  '    x.send();'+
  '  }, 1);'+
  '</script></body></html>';

procedure TForm2.FormCreate(Sender: TObject);
begin
  WebBrowser1.Navigate('http://127.0.0.1:'+IntToStr(http.Bindings[0].Port)); //9999
end;

procedure TForm2.httpCommandGet(AContext: TIdContext;
  ARequestInfo: TIdHTTPRequestInfo; AResponseInfo: TIdHTTPResponseInfo);
begin
  AResponseInfo.ContentText := doc;
  AResponseInfo.ContentType := 'text/html';
  AResponseInfo.Expires := 1;
end;

procedure TForm2.Timer1Timer(Sender: TObject);
begin
  Timer1.Enabled := False;  // Disable the timer because setting Active=false can take some time
  http.Active := False;
  http.Active := True;
  Timer1.Interval := Random(200)+1;
  Label1.Caption := 'Alive for '+IntToStr(Timer1.Interval)+' ms';
  Timer1.Enabled := True;
end;

end.

Answer 1

A long delay in my response here, as I just found this long-lost question again while investigating an unrelated race.

The patch for TIdThread.Cleanup() in Indy rev 5518 has resolved this race condition.