2

I have created a TIdHTTPServer based web server that runs on Windows. Indy version is very recent (Jan 2015). I have MaxConnections set to 1000, and have developed a mechanism to restart the web server when DoMaxConnections is called.

Once an hour I log the connection count:

TIdThreadSafeList(IdHTTPServer1.Contexts).count;

For the most part the connection count is always increasing. Once in a while I will see it decrease (say from 525 to 520). But the overall trend is increasing. After 5 days or so it gets to 1000 and the server resets (I understand 1000 is relatively small, but don't think it is germane to the subject).

Even on weekends, when traffic is lower, it still increases. I would have thought it would decrease at times of lower traffic. Why are some of these connections not closing?

I do netstat -an and see a lot of TIME_WAIT status.

Other relevant source code:

ServerIOHandler := TIdServerIOHandlerSSLOpenSSL.Create(self);
ServerIOHandler.SSLOptions.CertFile := 'C:\xxxxxx.crt';
ServerIOHandler.SSLOptions.KeyFile := 'C:\xxxxxx.key';
ServerIOHandler.SSLOptions.RootCertFile := 'C:=xxxxxx.crt';
ServerIOHandler.SSLOptions.Method := sslvSSLv23;
ServerIOHandler.SSLOptions.Mode := sslmServer;


IdHTTPServer1 := TIdHTTPServer.Create;
IdHTTPServer1.MaxConnections := 1000;
IdHTTPServer1.AutoStartSession := True;
IdHTTPServer1.KeepAlive := True;
IdHTTPServer1.SessionState := True;
IdHTTPServer1.OnCommandGet := MainGet;
idHttpServer1.ParseParams := True;

idHttpServer1.IOHandler := ServerIOHandler;
idHttpServer1.Bindings.Add.Port := 80;
idHttpServer1.Bindings.Add.Port := 443;
IdHTTPServer1.Active := True;

Update I thought it would be useful to add some netstat statistics.

With the server running for 24+ Hours:

Contexts Count: 587
netstat ESTABLISHED from external to port 80:580
netstat TIME_WAIT   from external to port 80:819

I then restarted my Indy Service (did not reboot windows server however):

Contexts Count: 60
netstat ESTABLISHED from external to port 80:49
netstat TIME_WAIT   from external to port 80:797

Update 2 - Request was made to show the MainGet procedure. There is too much code to feasibly do this. But what I have done is to provide snippets of code that could shed light on problem. Below is a list of some of the calls I am making (under different circumstances to improve performance).

AResponseInfo.ContentText := filetostring('c:\.....');
AResponseInfo.ResponseNo
Aresponseinfo.RawHeaders.Add('Connection:close');
Aresponseinfo.CustomHeaders.Add('Keep-Alive: timeout=30');
Aresponseinfo.Connection:='keep-alive';
AResponseInfo.ContentStream := MemoryStream;
AResponseInfo.Redirect('xxxxx');
AResponseInfo.ServeFile(AContext,mFile);
AResponseinfo.CacheControl:='max-age=1209600';

Update 3 - provide additional information of my the MainGet function. I am keeping track of the number of times the function is called and the number of times that it exits. The value of iGlobalGetStart and iGlobalGetFinish are always very close (certainly way less than number of contexts that are active).

  procedure TMyWebUnit.MainGet(AContext: TIdContext;
    ARequestInfo: TIdHTTPRequestInfo; AResponseInfo: TIdHTTPResponseInfo);
  try
    InterlockedIncrement(iGlobalGetStart);
    .... all of my logic
  finally
    InterlockedIncrement(iGlobalGetFinish);
  end;

Update 4 - I have implemented additional tracking code. I have created my own class descended from TIdServerContext. In it I set the date/time it is created, and also have a date/time member called DatetimeLastUsed which will get updated in oncommandget. Here is the class:

type
  TEAIdServerContext = class(TIdServerContext)
    DateTimelastUsed,DateTimeCreated: TDateTime;
    constructor Create(AConnection: TIdTCPConnection; AYarn: TIdYarn; AList: TIdContextThreadList = nil); override;
    destructor Destroy; override;
  end;

I set the IdHTTPServer1.ContextClass to this new class: IdHTTPServer1.ContextClass := TEAIdServerContext;

In my OnCommandGet, the first thing I do is to set the date/time of DateTimeLastUsed to the current time:

TEAIDServerContext(AContext).DateTimeLastUsed := Now;

I also have a function which will list all contexts:

procedure SayContexts();
var
  i: Integer;
  mstring: String;
  mList: TList;
begin

  mList := IdHTTPServer1.Contexts.LockList;
  mstring := 'Contexts:'+inttostr(mlist.Count);
  with mList do try
    for i := 0 to count -1 do begin
      mString := mString +
        datetimetostr(TEAIDServerContext(mList[i]).datetimecreated)+'; last used:'+datetimetostr(TEAIDServerContext(mList[i]).datetimeLastUsed)
    end;
  end;
  IdHTTPServer1.Contexts.UnLockList;
  ....
end;

The connections that are remaining open are never getting to the oncommandget code because their datetimelastused field is null. Is this normal?

M Schenkel
  • 6,294
  • 12
  • 62
  • 107
  • 1
    A single malicious client could open [up to 64K](http://stackoverflow.com/a/1806033/80901) connections to your server. So I would first do check where the connections originate - maybe a client is stuck in a loop and re-connects – mjn Jul 08 '15 at 14:25
  • When doing netstat nothing looked out of the ordinary. But I will check again. And there is never a dramatic increase in connections; it is always very gradual. – M Schenkel Jul 08 '15 at 14:55
  • 1
    Is the netstat connection count value close to the context count? If the netstat connection count is much lower then I would suspect your program does not always release the context when the connection is closed – mjn Jul 08 '15 at 15:01
  • Yes - see update I added. – M Schenkel Jul 08 '15 at 17:00
  • This does sound like client contexts are not being allowed to terminate correctly. Please show the `MainGet()` code. – Remy Lebeau Jul 08 '15 at 18:32
  • @RemyLebeau See Update2. Not possible to show all code. But I went through and have pasted different lines of code where I was setting various AReaponseInfo properties aside from ContentText and ContentStream. Of particular interest, now that I see it, is the CacheControl max-age:1209600. Could this be it? – M Schenkel Jul 08 '15 at 18:51
  • @MSchenkel: None of the snippets you have problem would cause the problem. However, yo uare doing some odd things. Why are you sending a file using `ContextText` and also setting `ContextStream`? Why are you adding a `Connection:close` header but then setting the `Connection` property to `keep-alive`? Why are you calling `Redirect()` and `ServeFile()` together? I think you probably just posted different sections of code together instead of keeping them separated. And no, `CacheControl` has no effect on connection lifetimes. – Remy Lebeau Jul 08 '15 at 18:57
  • 1
    @MSchenkel: What I am more interested in seeing is not how you are handling `AResponseInfo`, but rather how you are handing errors in particular. More specifically, if you happen to have any `try/except` blocks that are discarding Indy exceptions instead of letting `TIdHTTPServer` handle them. It is hard to diagnose problems like this when you do not produce an [MCVE](http://stackoverflow.com/help/mcve) to reproduce the problem. – Remy Lebeau Jul 08 '15 at 18:58
  • @RemyLebeau Yes - I added most of the ways I was modifying AResponseInfo properties. These certainly are not all done within the same procedure. I will examine more of my functions. Would it be possible to illustrate a situation of "discarding Indy exceptions instead of letting TIdHTTPServer handle them". Thanks too for referencing the MCVE page - I will use that in the future. – M Schenkel Jul 08 '15 at 19:16
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/82735/discussion-between-m-schenkel-and-remy-lebeau). – M Schenkel Jul 08 '15 at 19:25
  • @RemyLebeau I want to point out that in **MainGet** I keep track how many times the function is entered and how many times it completes: **InterlockedIncrement(iGlobalGetStart);** and ** InterlockedIncrement(iGlobalGetFinish);**. And these numbers are off by only 3 (while there are 250 connections). So from what I can tell, any time this function is entered it is also properly exiting. But could there still be situations of "discarding Indy Exceptions"? – M Schenkel Jul 10 '15 at 15:27
  • Are you wrapping your interlocked calls with a `try/finally` block? If not, then an uncaught/re-raised exception would bypass `InterlockedIncrement(iGlobalGetFinish)`. So this could be an indication that you experienced 3 exceptions rather than 3 stuck connections. And the only way to "discard an exception" is to **catch it** and not re-raise it. – Remy Lebeau Jul 10 '15 at 17:07
  • @RemyLebeau - See **Update 3** – M Schenkel Jul 10 '15 at 20:44
  • @MSchenkel: `InterlockedIncrement(iGlobalGetStart)` should be outside of the `try`. But whatever. All you are really doing is tracking how many HTTP requests are being made, not how many connections are being made/closed. But without seeing the rest of the code, there is just no way to diagnose your problem. Something is getting stuck, but there is no way for anyone here to see what that could actually be. – Remy Lebeau Jul 10 '15 at 21:28
  • @RemyLebeau So as a starting point should I map the OnConnect, ondisconnect, onsessionend and onsessionstart events? All the examples I have seen don't get into these events/methods. I was under impression indy would take care of all the clean up. Obviously I am not digging deep enough. Is there a way in the "OnConnect" that I can see the calling document (ARequestInfo.Document)? Maybe I can make a construct that keeps track of connects and disconnects and link this to the problematic ARequestInfo.Document. – M Schenkel Jul 10 '15 at 22:11
  • @MSchenkel: The number of `OnConnect/OnDisconnect` pairs should match the number of items in the `Contexts` list. The `OnSession...` events won't tell you anything useful in this situation. No, the document info is not available in the `OnConnect` event, as no HTTP request has been transmitted yet at that time. You have to use the `OnCommand...` events to access request information. – Remy Lebeau Jul 10 '15 at 22:19
  • @RemyLebeau - I am back to solving this problem. I have some new information. I am finding the number of OnDisconnect calls is exceeding the number of OnConnect calls. I have created two integer variables to keep track of these calls. The difference is typically close to the number of Contexts count. How could the number of OnDisconnects be greater than the number of OnConnects? – M Schenkel Sep 09 '15 at 20:07
  • @MSchenkel: "*the number of OnDisconnect calls is exceeding the number of OnConnect calls*" - the `OnConnect` and `OnDisconnect` events are triggered in the context of the same worker thread. You can't have an `OnDisconnect` event without a preceding `OnConnect` event. Unless you are dynamically assigning the event handlers *after* the server is already active, where a client could connect before the `OnConnect` handler has been assigned. The more likely culprit is your management of the integer variables, they are probably not being accessed in a thread-safe manner and thus get out of sync. – Remy Lebeau Sep 09 '15 at 20:41
  • @MSchenkel: Remember that `TIdTCPServer` is multi-threaded. You can have multiple `OnConnect` events, and multiple `OnDisconnect` events, running in parallel at the same time. Your event handlers need to be thread-safe. – Remy Lebeau Sep 09 '15 at 20:43
  • @RemyLebeau - I thought by using InterlockedIncrement(iOnConnect) it would be thread safe. All I am doing in the OnDisconnect and OnConnect is incrementing the values wrapped in "interlockedincrement". – M Schenkel Sep 09 '15 at 20:48
  • @MSchenkel: Are the variables aligned on 32-bit memory boundaries? That is a requirement for proper atomic access: "*The variable pointed to by the `Addend` parameter **must** be aligned on a 32-bit boundary; otherwise, this function will **behave unpredictably** on multiprocessor x86 systems and any non-x86 systems.*". And are you using the interlocked API to read the variable values as well? If not, you could be reading junk values if you are reading at the time they were being incremented. Maybe consider using Indy's `TIdThreadSafeInteger` class in the `IdThreadSafe` unit. – Remy Lebeau Sep 09 '15 at 21:02
  • @RemyLebeau Not sure. Using Delphi 2007 and the variable is defined as "integer". Will do more research on this. No - I am not using "interlocked" when I read the variable; only when it is incremented. I will modify as you say I could be reading junk values. – M Schenkel Sep 09 '15 at 23:08
  • @RemyLebeau when reading the values, if I don't use the interlocked API, would I expose myself to reading "junk"? Or perhaps a value that may be off a bit? – M Schenkel Sep 09 '15 at 23:31
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/89203/discussion-between-remy-lebeau-and-m-schenkel). – Remy Lebeau Sep 09 '15 at 23:43
  • @RemyLebeau Did more research on this. Came across this link: http://codeverge.com/embarcadero.delphi.winsock/quietly-handling-exceptions-in-ind/1074364 on which you responded. I had a ton of places where I was doing exception handling without re-raising. I did not understand this is what is meant by this: **if you happen to have any try/except blocks that are discarding Indy exceptions instead of letting TIdHTTPServer handle them.** I did not know I was in fact discarding them. I have modified my code to re-raise **EIdException** exceptions. Will let you know if this works. – M Schenkel Sep 11 '15 at 20:32
  • @RemyLebeau Did some more on this. See Update 4. I know this entire question is not well presented, with no MCVE. Long story short: the connections that are not closing are never making it to **oncommandget** handler. – M Schenkel Sep 14 '15 at 19:48
  • @MSchenkel: are those clients sending any commands at all? You should consider setting a `ReadTimeout` property value in the `OnConnect` event to help reduce Denial-of-Service issues. Is the `OnHeadersAvailable` event being triggered for these clients? Are they sending commands other than `GET`, `POST`, or `HEAD`? Other commands would go to the `OnCommandOther` event instead of the `OnCommandGet` event. – Remy Lebeau Sep 14 '15 at 20:18
  • @RemyLebeau - I have added a **oncommandOther**, but this is never called. The ones that do go through **OnCommandGet** are always either Get or Post (which is expected). I did have **idhttpserver.ReadTimeOut set to 5000. And I have also added **AContext.connection.socket.ReadTimeOut := 5000;** to the OnConnect. I will look into **OnHeadersAvailable** – M Schenkel Sep 14 '15 at 21:08
  • `TIdHTTPServer` does not have a `ReadTimeout` property of its own. You have to set the `ReadTimeout` on the individual `AContext` connections. – Remy Lebeau Sep 14 '15 at 21:10
  • @RemyLebeau Added `OnHeadersAvailable`. And this is being called for all standards `Gets`. I will let it run for 30 minutes or so and see if setting `ReadTimeOut` to 5000 will keep connections from increasing. – M Schenkel Sep 14 '15 at 21:20
  • @RemyLebeau It appears the `ReadTimeOut` has made a HUGE difference: All connections seem to be releasing now! Yes - I was mistaken in prior comment. It was not `ReadTimeOut` of `TIdHttpServer`. But instead it was of a `TIdHttp` instance. – M Schenkel Sep 14 '15 at 21:42
  • @RemyLebeau - Remy - Setting a value for `ReadTimeOut` in the `OnConnect` handler has solved this problem. Can you post an answer so that I can give you credit? I am still perplexed at this however. And wonder why more people are not having this problem (unless they know enough to set the ReadTimeOut - but I have never seen this as part of the many source code demos I have seen). – M Schenkel Sep 22 '15 at 15:49

0 Answers0