Skip to content

Commit fd2cc93

Browse files
committed
include GetResponse() in using and inlined it so we can more aggresively cancel on the waithandles in the case of exceptions too
1 parent 8d80683 commit fd2cc93

File tree

1 file changed

+34
-33
lines changed

1 file changed

+34
-33
lines changed

src/Elasticsearch.Net/Connection/HttpConnection.cs

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -189,9 +189,12 @@ private static void TimeoutCallback(object state, bool timedOut)
189189
(state as WebRequest)?.Abort();
190190
}
191191

192-
public virtual async Task<ElasticsearchResponse<TReturn>> RequestAsync<TReturn>(RequestData requestData, CancellationToken cancellationToken) where TReturn : class
192+
public virtual async Task<ElasticsearchResponse<TReturn>> RequestAsync<TReturn>(RequestData requestData,
193+
CancellationToken cancellationToken) where TReturn : class
193194
{
194195
var builder = new ResponseBuilder<TReturn>(requestData, cancellationToken);
196+
WaitHandle apmWaitHandle = null;
197+
RegisteredWaitHandle apmTaskTimeout = null;
195198
try
196199
{
197200
var data = requestData.PostData;
@@ -201,39 +204,39 @@ public virtual async Task<ElasticsearchResponse<TReturn>> RequestAsync<TReturn>(
201204
if (data != null)
202205
await PostRequestAsync(requestData, cancellationToken, request, data);
203206
requestData.MadeItToResponse = true;
207+
//http://msdn.microsoft.com/en-us/library/system.net.httpwebresponse.getresponsestream.aspx
208+
//Either the stream or the response object needs to be closed but not both although it won't
209+
//throw any errors if both are closed atleast one of them has to be Closed.
210+
//Since we expose the stream we let closing the stream determining when to close the connection
211+
212+
var apmGetResponseTask = Task.Factory.FromAsync(request.BeginGetResponse, request.EndGetResponse, null);
213+
apmWaitHandle = ((IAsyncResult) apmGetResponseTask).AsyncWaitHandle;
214+
apmTaskTimeout = RegisterApmTaskTimeout(apmGetResponseTask, request, requestData);
215+
216+
var response = (HttpWebResponse) (await apmGetResponseTask.ConfigureAwait(false));
217+
builder.StatusCode = (int) response.StatusCode;
218+
builder.Stream = response.GetResponseStream();
219+
if (response.SupportsHeaders && response.Headers.HasKeys() && response.Headers.AllKeys.Contains("Warning"))
220+
builder.DeprecationWarnings = response.Headers.GetValues("Warning");
221+
// https://github.com/elastic/elasticsearch-net/issues/2311
222+
// if stream is null call dispose on response instead.
223+
if (builder.Stream == null || builder.Stream == Stream.Null) response.Dispose();
224+
if (apmWaitHandle != null) apmTaskTimeout?.Unregister(apmWaitHandle);
204225
}
205-
await GetResponseAsync(requestData, request, builder);
206226
}
207227
catch (WebException e)
208228
{
229+
if (apmWaitHandle != null) apmTaskTimeout?.Unregister(apmWaitHandle);
209230
HandleException(builder, e);
210231
}
211-
232+
catch
233+
{
234+
if (apmWaitHandle != null) apmTaskTimeout?.Unregister(apmWaitHandle);
235+
throw;
236+
}
212237
return await builder.ToResponseAsync().ConfigureAwait(false);
213238
}
214239

215-
private static async Task GetResponseAsync<TReturn>(RequestData requestData, HttpWebRequest request, ResponseBuilder<TReturn> builder)
216-
where TReturn : class
217-
{
218-
//http://msdn.microsoft.com/en-us/library/system.net.httpwebresponse.getresponsestream.aspx
219-
//Either the stream or the response object needs to be closed but not both although it won't
220-
//throw any errors if both are closed atleast one of them has to be Closed.
221-
//Since we expose the stream we let closing the stream determining when to close the connection
222-
223-
var apmGetResponseTask = Task.Factory.FromAsync(request.BeginGetResponse, request.EndGetResponse, null);
224-
var getResponseCancellationHandle = RegisterApmTaskTimeout(apmGetResponseTask, request, requestData);
225-
226-
var response = (HttpWebResponse) (await apmGetResponseTask.ConfigureAwait(false));
227-
builder.StatusCode = (int) response.StatusCode;
228-
builder.Stream = response.GetResponseStream();
229-
if (response.SupportsHeaders && response.Headers.HasKeys() && response.Headers.AllKeys.Contains("Warning"))
230-
builder.DeprecationWarnings = response.Headers.GetValues("Warning");
231-
// https://github.com/elastic/elasticsearch-net/issues/2311
232-
// if stream is null call dispose on response instead.
233-
if (builder.Stream == null || builder.Stream == Stream.Null) response.Dispose();
234-
getResponseCancellationHandle.Unregister(((IAsyncResult) apmGetResponseTask).AsyncWaitHandle);
235-
}
236-
237240
private static async Task PostRequestAsync(RequestData requestData, CancellationToken cancellationToken, HttpWebRequest request,
238241
PostData<object> data)
239242
{
@@ -256,14 +259,12 @@ private void HandleException<TReturn>(ResponseBuilder<TReturn> builder, WebExcep
256259
{
257260
builder.Exception = exception;
258261
var response = exception.Response as HttpWebResponse;
259-
if (response != null)
260-
{
261-
builder.StatusCode = (int)response.StatusCode;
262-
builder.Stream = response.GetResponseStream();
263-
// https://github.com/elastic/elasticsearch-net/issues/2311
264-
// if stream is null call dispose on response instead.
265-
if (builder.Stream == null || builder.Stream == Stream.Null) response.Dispose();
266-
}
262+
if (response == null) return;
263+
builder.StatusCode = (int)response.StatusCode;
264+
builder.Stream = response.GetResponseStream();
265+
// https://github.com/elastic/elasticsearch-net/issues/2311
266+
// if stream is null call dispose on response instead.
267+
if (builder.Stream == null || builder.Stream == Stream.Null) response.Dispose();
267268
}
268269

269270
void IDisposable.Dispose() => this.DisposeManagedResources();

0 commit comments

Comments
 (0)