AV with multiple HTTP Client failures

[DougB]

I've created a sample application that sends requests to a HTTP server.  I noticed that when the server begins returning 'connection refused' errors, I eventually get an AV in the RTC code.

To reproduce this, you don't even need to connect to a server, just run the app and click on the 'Start' button.  Connections will be created at a high rate and eventually (within 5 minutes) you will see an AV.

This is a contrived test, of course, but it illustrates a problem when a flood of connections is created and they end up failing.

You can download the code here:

http://ge.tt/6QATJDr1/v/0

I'm assuming that I'm doing something wrong, but I did my best to follow your instructions in a previous thread for using a QuickJob to post a disconnect event.

Thanks,
Doug

[D.Tkalcec (RTC)]

Thank you for the example Project. Essentially, the reason for the AV was that the event from which you were posting the background Job did NOT always have enough time to complete execution before the thread which was executing the background job managed to close the connection and destroy the connection component. On top of that, because the OnConnectError event is being called after the OnResponseAbort event and your OnConnectError event was changing a private variable of your THttpClient object, which could already have been destroyed from the background job thread at that time, memory was being overwritten, which can cause more AVs at other places.

Fortunately, the solution is rather simple. Instead of posting a Job to a separate background thread and using the DisconnectNow method to close the connection before destroying it (I know, this is what I've recommended - my bad), which does not really help if a RTC event is still running (because it did not have enough time to complete execution), you can destroy your THttpClient instance directly from the OnResponseDone, OnResponseAbort and OnResponseReject events if you use the fClient.Release method instead of fClient.Free in your THttpClient destructor. The "Release" method on the TRtcHttpClient component will send a message to the connection components background thread to destroy the component as soon as posslble, rather than destroy it immediately, which is why the "Release" method it safe for use from inside RTC events.

You will also need to remove your OnConnectError, OnConnectLost and OnConnectFail events, or change their implementation to avoid using your THttpClient object after you have already destroyed it, or you will start getting AVs because of them.

Here is a modified version of your Example Project, with all the changes listed above:
http://www.realthinclient.com/test/DougBClient1.zip

Best Regards,
Danijel Tkalcec

[DougB]

Awesome, thanks!

Is there any way to get a notification from the RTC thread pool when the client has actually been released (or could you point me to the code where I could implement such a feature)?

I ask because ultimately, I want to have a pool of THttpClient objects that are pre-allocated and that I can reuse, however I won't know when I can reuse it until the connection is actually freed).  I can stick with the existing scenario, but I'd prefer to use a pool, because I typically like to pre-allocate resources whenever possible...

Thanks again,
Doug

[D.Tkalcec (RTC)]

If you need to know when the TRtcHttpClient component is being destroyed, since you are creating TRtcHttpClient components dinamically in code, you can implement your own TRtcHttpClient descendant and override the component destructor. But you should NOT use the component after you have called Release, because calling Release will immediately make the component unusable. And secondly, after you call Release, depending on the number of currently active threads and the component state, it could take anywhere from a few milliseconds to a few seconds for the component to actually get destroyed by the RTC thread pool.

On the other hand, there is no reason why you should NOT be able to implement a pool of THttpClient objects, without knowing when the TRtcHttpClient component is being destroyed by the background thread. Instead of clearing the THttpClient pointer from the TRtcHttpCLient's Info object and destroying it, you can use the OnResponseDone, OnResponseAbort and OnResponseReject events to put your THttpClient object into an object pool and re-use it. The Send method on your THttpClient class can safely be used from anywhere, as long as the two containing components (TRtcHttpClient and TRtcDataRequest) are in-tact.

Best Regards,
Danijel Tkalcec

[DougB]

Ok, I understand I can call HttpClient.Send and new requests will be posted to the queue and handled in order of being received.  For the same connection/destination server, this is fine.  However, if the next request is for a different server, then the connection must be disconnected before a new message is sent, correct?.

Therefore, on the OnResponseDone, OnResponseAbort and OnResponseReject events I want to:

1. Disconnect the client from the server (to free the resource on the server) without freeing the client (e.g., HttpClient.Free)

2. Return the HttpClient to the pool for reuse (immediately, and potentially to a different server address).

Obviously, #2 cannot happen if the client is still in the process of disconnecting before a new request is sent, correct?  And I still can't call DisconnectNow from within one of these events?

Sorry for all the questions, but I'm getting very close here and my understanding of how RTC works is improving.  I also really appreciate your responsiveness and the feedback.

Thanks,
Doug 

[D.Tkalcec (RTC)]

You can call Disconnect from within the RTC event, forcing the connection to be closed after the event completes execution, but ... by forcing a disconnect before placing the component back into your pool, you will be wasting 95% of the advantage you would get from re-using the components, because the biggest advantage of re-using components is re-using an open TCP/IP connection to the Server.

If you expect your Client to work with the same set of Servers, I would recommend you to keep the connection open after the OnResponseDone event and manage a separate pool for each ServerAddr:ServerPort pair. Otherwise, you won't gain much by using a pool VS your current scenario where you are simply creating and destroying the components on-demand.

Best Regards,
Danijel Tkalcec

[DougB]

by forcing a disconnect before placing the component back into your pool, you will be wasting 95% of the advantage you would get from re-using the components, because the biggest advantage of re-using components is re-using an open TCP/IP connection to the Server.

If you expect your Client to work with the same set of Servers, I would recommend you to keep the connection open after the OnResponseDone event and manage a separate pool for each ServerAddr:ServerPort pair. Otherwise, you won't gain much by using a pool VS your current scenario where you are simply creating and destroying the components on-demand.

Agreed.  And this is what I was planning, but I wanted to know when it was safe to disconnect the component if I needed to in some other scenarios.

A few more questions:

1. Where is the appropriate place to perform a DNS lookup (to get the IP address for a URL) using the components?

2. In the OnResponseReject & OnResponseAbort event handlers, since I'm not implementing OnConnectError (because my HttpClient instance may not be available when that event is called), is there a method I can call to determine what the error condition was (e.g., so I know if it was a socket error or internal error, like a bad Client.Addr)?  I've notice the internal connection provider 'Con' has a GetLastErrorText method, but that object isn't available from the TRtcHttpClient.

Thanks,
Doug

[D.Tkalcec (RTC)]

There is no special "DNS-lookup" component in the RTC SDK. You simply use domain names in the ServerAddr property. All low-level APIs supported by the RTC SDK will do the DNS lookup automatically. DNS lookup will be done by the low-level API before opening a connection, unless the low-level API has already cached the results from a previous connection attempt, in which case the last cached IP will be used. Caching is done automatically by the low-level APIs, so there is nothing you need to do about that.

Best Regards,
Danijel Tkalcec

[DougB]

Excellent, thanks!

Any comments on #2 regarding error messages?

Doug

[D.Tkalcec (RTC)]

If you want to know what exactly cuaused the aborted response, instead of destroying your THttpClient object and RTC components from the OnResponseAbort event, you will need to postpone that action and do it from the OnConnectError, OnConnectLost or OnConnectFail events, since you only have access to the error message from the OnConnectError event, which is being called after the OnResponseAbort event.

The "GetLastErrorText" method, which you have found on the low-level Socket connection provider class, is used by the connection provider to create the Exception object passed on to the OnConnectError event.

Best Regards,
Danijel Tkalcec