Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
TidHTTPServer and Orphaned Connection Threads
#1
So I've got an HTTP Service and after about 25,000-35,000 connections in a live environment, I start to get "EOutofMemory" errors and eventually the OnListenException event starts getting triggered with "Thread creation error: Not enough storage is available to process this command". 

Eventually the service just can't accept any new connections and we get other evidence if resource limitation issues in the main service thread and anything that attempts to write to the disk. 

Now there are plenty of EIdOSSLAcceptError and EIdSocketError (usually 10054, 10053 or 10060) triggered in the generic HTTP Exception handler. I've recently updated from 1.0.1t to 1.0.2s on the openssl side to see if it mitigates some of these ssl protocol errors. Anyhow, trapped inside my OnCommandGet Handler, the most common exception (Socket Error #10054. Connection reset by peer.) is around a GET request that serves up an image via SmartServeFile. But it's not specific to any one image and it's intermittent and not something I've been able to trigger in the development environment, we figure the client just drops connection before SmartServe can complete and this is what it looks like when it happens. Most of the 'heavy lifting' for the service happens in a POST process.

Now I've tried creating some exceptions in my OnCommandGet handler (sending bad data) to see if I'm cleaning up in my try/finally and exceptions and it all looks good. Eurekalog doesn't report any leaked memory and my thread count stays stable. 

In the live environment, I see the service is steadily growing in thread count. Seems to die somewhere around 1500 threads after about 6-10hrs depending on traffic, however plenty of hardware resources left (RAM and SSD). A simple restart of the service works just fine. So this implies there's a bunch of orphaned threads. Looking at process monitor, I can see hundreds of threads what would likely have started as CommandGet connection threads, still existing from HOURS ago. CPU utilization is very low, and the application handling these thousands of connections doesn't use up much cycles... so it doesn't appear these threads are stuck in any kind of loop. The Stack for all of these 'old' threads just shows:

ntoskrnl.exe!KeSynchronizeExecution+0x5bd6
ntoskrnl.exe!KeWaitForMultipleObjects+0x109d
ntoskrnl.exe!KeWaitForMultipleObjects+0xb3f
ntoskrnl.exe!KeWaitForMutexObject+0x377
ntoskrnl.exe!KeQuerySystemTimePrecise+0x881
ntoskrnl.exe!ObDereferenceObjectDeferDelete+0x28a
ntoskrnl.exe!KeWaitForMultipleObjects+0x1284
ntoskrnl.exe!KeWaitForMultipleObjects+0xb3f
ntoskrnl.exe!KeWaitForMutexObject+0x377
ntoskrnl.exe!NtWaitForSingleObject+0xf8
ntoskrnl.exe!_setjmpex+0x68a3
wow64cpu.dll!TurboDispatchJumpAddressEnd+0x540
wow64cpu.dll!TurboDispatchJumpAddressEnd+0x3a5
wow64.dll!Wow64KiUserCallbackDispatcher+0x4151
wow64.dll!Wow64LdrpInitialize+0x120
ntdll.dll!LdrInitializeThunk+0x16d
ntdll.dll!LdrInitializeThunk+0xe

None of which is my windows service (obviously), but it suggests the thread(s) are still alive.

I'm wondering how I can dig deeper into these seemingly orphaned threads to figure out how they got orphaned. Even with the EIdOSSLAcceptError and EIdSocketError's the number of those that occur, do not accumulate to the number of threads. And yes, I'm re-raising EIdExceptions in my handlers. 

Any ideas would be appreciated. 

Indy version is 10.6.1.5182 on XE7. 

Basic service structure is in the OnCommandGet is...

Code:
procedure TsvcThing.srvHTTPCommandGet(AContext: TIdContext; ARequestInfo: TIdHTTPRequestInfo; AResponseInfo: TIdHTTPResponseInfo);
var
  XMLDoc: IXMLDocument;
  adoConn: TADOConnection;
  adoQuery, adoQueryOther: TADOQuery;
begin
  try
    CoInitialize(nil);
    try
      if ARequestInfo.Command = 'GET' then
        //serve up the requested file
        AResponseInfo.SmartServeFile(AContext, ARequestInfo, Path + filename);

      if ARequestInfo.Command = 'POST' then
      begin
        adoConn := TADOConnection.Create(nil);
        adoQuery := TADOQuery.Create(nil);
        adoQueryOther := TADOQuery.Create(nil);

        try
          adoConn.ConnectionString := 'valid connection string';
          adoQuery.Connection := adoConn;
          adoQueryOther.Connection := adoConn;

          if not adoConn.Connected then
            adoConn.Connected := true;

          XMLDoc := LoadXMLData(ReadStringFromStream(ARequestInfo.PostStream, -1));
          try
            //perform various queries (Open and ExecSQL) based on XML request

            //set fields in XML response

            if error_processing_xml_or_data then
              AResponseInfo.ResponseNo := 400
            else
            begin
              AResponseInfo.ContentType := 'application/xml';
              AResponseInfo.ResponseNo := 200;
              AResposneInfo.ContentText := XMLDoc.XML.Text;
            end;
          finally
            XMLDoc.Active := False;
          end;
        finally
          FreeAndNil(adoQueryOther);
          FreeAndNil(adoQuery);
          FreeAndNil(adoConn);
        end;    
      end;
    finally
      CoUnintialize;
    end;
   except
     on E: Exception do
       if Log then
          WriteLog(E.Message);
     on EIdException do raise;
   end;
end;
Reply
#2
(06-27-2019, 08:02 PM)takyon_dg Wrote: Indy version is 10.6.1.5182 on XE7. 

That is a VERY old version of Indy.  The current version is 10.6.2.5503.  I strongly suggest you upgrade and see if the problem still continues.

(06-27-2019, 08:02 PM)takyon_dg Wrote: Basic service structure is in the OnCommandGet is...

I notice is there are no local variables being declared.  Are your ADO and XML variables really being shared across multiple firings of the OnCommandGet event?  That is not good, if you have multiple clients connected to your server at the same time.  The ADO and XML objects need to be local to each request, or cached in the TIdContext object so they can be reused in multiple requests from the same client.

Also, you are not releasing the XML object from memory before calling CoUninitialize(), so it will get released after COM has already been ripped away from behind its proverbial back.  You MUST release/nil COM interfaces before uninitializing COM.

For that matter, I would not suggest calling Co(Un)Initialize() inside of your OnCommandGet handler at all.  A better solution is to
  • create a new class derived from TIdThreadWithTask and override its virtual BeforeExecute() and AfterExecute() methods to call Co(Un)Initialize().
  • create an explicit TIdSchedulerOfThreadDefault or TIdSchedulerOfThreadPool object and assign your thread class to its ThreadClass property.
  • assign that Scheduler object to the TIdHTTPServer.Scheduler property before activating the server.

Reply
#3
Thanks Remy for responding.

I just threw that example of the structure together and didn't add in any of the "var" declarations. They are locally declared to OnCommandGet and not shared across connections/threads. (I'll edit the original to make that clear)

As for the XML object, my understanding is it doesn't need to be released as it's type IXMLDocument. Everything I've seen online about using it suggests it will get freed when it goes out of scope. I figured the most I could do is set Active to false which should be triggered via it's try-finally before the CoInit/CoUnInit are. Are you saying I should be setting XMLDoc := nil; after XMLDoc.Active := False?

With those pieces perhaps cleared out. I'm curious why you suggest using the scheduler. Is it simply better practice or do you believe it is related to my reported issue(s).

And yes, my next step was to upgrade Indy, though I'm cautious as I can't find a changelog and have a lot of applications that use indy and have left it as a last result hoping there aren't any significant functional changes that may force me to recompile. As I looked through the blog and repository, I couldn't see anything in the entries or bug list that suggested anything like my problem was fixed... and I suspect a lot doesn't get recorded there... is there not a itemized changelog somewhere?
Reply
#4
(06-30-2019, 10:38 PM)takyon_dg Wrote: As for the XML object, my understanding is it doesn't need to be released as it's type IXMLDocument.

Yes, it does need to be released (all COM interfaces do) before COM is uninitialized.  Either by letting the variable go out of scope sooner rather than later (ie, moving your XML processing logic into another function that is called after CoInitialize() and before CoUninitialize()), or by explicitly setting the variable to nil.  On Windows, IXMLDocument internally uses the MSXML library by default, and MSXML is based on COM objects.

(06-30-2019, 10:38 PM)takyon_dg Wrote: Everything I've seen online about using it suggests it will get freed when it goes out of scope.

Yes, but in your code example, that happens AFTER CoUninitialize() is called, which is a bad thing.

(06-30-2019, 10:38 PM)takyon_dg Wrote: I figured the most I could do is set Active to false which should be triggered via it's try-finally before the CoInit/CoUnInit are.

Deactivating the document is a different operation then releasing the document.

(06-30-2019, 10:38 PM)takyon_dg Wrote: Are you saying I should be setting XMLDoc := nil; after XMLDoc.Active := False?

Yes.  As well as any other IXML... interface variables that are still in scope when CoUninitialize() is called.

(06-30-2019, 10:38 PM)takyon_dg Wrote: I'm curious why you suggest using the scheduler. Is it simply better practice or do you believe it is related to my reported issue(s).

My suggestion has nothing to do with your particular issue.  It is just an enhancement, given how many connections you are processing. Indy TCP servers use 1 thread per TCP connection.  Thread creation and destruction are expensive tasks for an OS, so you might consider using TIdSchedulerOfThreadPool to pool and reuse connection threads over time to lessen the overhead.  By default, Indy TCP servers use TIdSchedulerOfThreadDefault, which creates a new thread on each new TCP connection, and terminates the thread on disconnect.

COM should only be initialized once per thread, so it makes sense to hook into the thread startup/shutdown logic to (un)initialize COM one time per thread.  Even if you don't use thread pooling, I would still recommend this approach, because it moves the Co(Un)Initialize() calls higher up on the call stack, outside of your OnCommandGet event handler, ensuring that COM is initialized before OnCommandGet is called, and is uninitialized after OnCommandGet exits and all of your local IXML... variables go out of scope.

(06-30-2019, 10:38 PM)takyon_dg Wrote: And yes, my next step was to upgrade Indy, though I'm cautious as I can't find a changelog ... is there not a itemized changelog somewhere?

No, there is not.  You would have to look at the individual SVN/GIT checkin comments.

Although, Indy's website does contain a blog that has a Changelog category, which I post to whenever I make noteworthy changes to Indy that may affect user code and require explanation. But I don't post to it for smaller changes that only affect Indy internally.

Reply
#5
Thanks again Remy, you're a hell of an asset to (at least) Delphi programmers online.

I've upgraded Indy, so we'll see how that goes. I may have more questions later about the impact (if any) of some Socket Errors (related to SmartServeFile) and failed SSL handshaking attempts. 

I'm also setting the XMLDoc to nil after Active := False, which I now imagine will address the leak/'orphaned' threads.

And thanks for the efficiency suggestion, the way you explained it makes total sense and I'll likely implement it once I have a handle on the core issue. I wonder if you can think of any reason why I wouldn't want to use Thread Pooling? Less reliable? More overhead in certain use-cases?
Reply
#6
(07-03-2019, 01:54 PM)takyon_dg Wrote: I'm also setting the XMLDoc to nil after Active := False, which I now imagine will address the leak/'orphaned' threads.

That had nothing to do with your orphaned thread issue. It was simply a bug in your code related to how you manage COM objects.

Reply
#7
(07-04-2019, 12:22 AM)rlebeau Wrote:
(07-03-2019, 01:54 PM)takyon_dg Wrote: I'm also setting the XMLDoc to nil after Active := False, which I now imagine will address the leak/'orphaned' threads.

That had nothing to do with your orphaned thread issue.  It was simply a bug in your code related to how you manage COM objects.
You're right (I thought it might create a leak).

New Indy is in there (6.2.5499), and that line. Same problem persists.

Any suggestions?

What I was thinking was I would track threadId's, their HTTP request and then in some other event handler, remove them from the list if they are properly closed and log the orphaned ones on a timer... I guess what I'm asking is, in terms of the TidHTTPServer events, is there (and which) one that gets triggered after the OnCommandGet (connection thread) is properly terminated/exited? I see OnContextCreated, but not sure which one might happen after. or would I have to gain access to the TidSchedulerOfThreadDefault and assign a method to the AfterExecute event?

Also, is there a straight-forward way to pull the HTTP header User-Agent from TidContext in OnException? I'm trying to isolate if a lot of my socket errors (including SSL handshake issues) are specific to an client OS or http control version. All the exceptions I have here seem to occur before OnCommandGet which I imagine means the datastream has not been identified/parsed as HTTP. My past attempts showed nothing in the input buffer to record and I wasn't sure what would happen if I try to use any of the read functions (AContext.Connection.Socket....)
Reply
#8
(07-04-2019, 03:41 PM)takyon_dg Wrote: You're right (I thought it might create a leak).

Releasing the XMLDoc after CoUninitialize() is called is undefined behavior, anything could happen.

(07-04-2019, 03:41 PM)takyon_dg Wrote: New Indy is in there (6.2.5499)

The current version is 10.6.2.5505.

(07-04-2019, 03:41 PM)takyon_dg Wrote: and that line. Same problem persists.

Then please create a minimal example that reproduces the same issue and then post the code.

(07-04-2019, 03:41 PM)takyon_dg Wrote: What I was thinking was I would track threadId's, their HTTP request and then in some other event handler, remove them from the list if they are properly closed and log the orphaned ones on a timer... I guess what I'm asking is, in terms of the TidHTTPServer events, is there (and which) one that gets triggered after the OnCommandGet (connection thread) is properly terminated/exited?

There is no event for that specific use-case. If you want to detect when a thread terminates, you need to derive a new class from TIdThreadWithTask and set the server's Scheduler.ThreadClass property, like I described earlier.

The OnDisconnect event is fired when the client connection is being disconnected and the thread that managed it is about to be cleaned up (terminated when using TIdSchedulerOfThreadDefault, put in the pool when using TIdSchedulerOfThreadPool).

(07-04-2019, 03:41 PM)takyon_dg Wrote: I see OnContextCreated, but not sure which one might happen after.

There isn't one.

(07-04-2019, 03:41 PM)takyon_dg Wrote: would I have to gain access to the TidSchedulerOfThreadDefault and assign a method to the AfterExecute event?

Yes. That would be the most accurate way to detect a thread terminating.

(07-04-2019, 03:41 PM)takyon_dg Wrote: Also, is there a straight-forward way to pull the HTTP header User-Agent from TidContext in OnException?

No, because the TIdHTTPRequestInfo object is not stored in TIdContext at all. And even if it were, it would get destroyed before the OnException event is fired.

In order to access any HTTP-related values from outside of the events that provide access to them, you will have to save the values yourself. For instance, you could use the TIdContext.Data property. Or, you can derive a new class from TIdServerContext and add whatever extra members you want to it, and then set the server's ContextClass property before activating the server. You can then type-cast any the server's TIdContext objects to your custom class when needed.

(07-04-2019, 03:41 PM)takyon_dg Wrote: I'm trying to isolate if a lot of my socket errors (including SSL handshake issues) are specific to an client OS or http control version.

Any errors you encounter during the SSL/TLS handshake will happen before any HTTP data is transmitted, since the connection has not been secured yet. But once the handshake is complete, any subsequent SSL/TLS errors would happen during HTTP I/O.

(07-04-2019, 03:41 PM)takyon_dg Wrote: All the exceptions I have here seem to occur before OnCommandGet which I imagine means the datastream has not been identified/parsed as HTTP.

A LOT of things happen before the OnCommand... events are fired. There is not a good way to know at which stage exactly (SSL/TLS handshake, reading/parsing HTTP headers, reading/parsing HTTP body).

(07-04-2019, 03:41 PM)takyon_dg Wrote: My past attempts showed nothing in the input buffer to record

OpenSSL has direct access to the socket connection and performs its own socket I/O. The only data that ever reaches the IOHandler's InputBuffer is the data that OpenSSL has decrypted.

(07-04-2019, 03:41 PM)takyon_dg Wrote: and I wasn't sure what would happen if I try to use any of the read functions (AContext.Connection.Socket....)

The socket has already been closed by the time the OnException event is fired, so whatever data would be in the IOHandler's InputBuffer would be what has already been read from the socket and decrypted. And if you try to read from the socket directly, that would be encrypted data.

Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)