Skip to content

Commit 9696381

Browse files
dnadobadnadoba
authored
[HTTP2Connections] Return if connection was idle before lease (#451)
1 parent d928cc8 commit 9696381

File tree

2 files changed

+74
-23
lines changed

2 files changed

+74
-23
lines changed

‎Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP2Connections.swift‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ extension HTTPConnectionPool {
424424

425425
// MARK: Leasing and releasing
426426

427-
mutating func leaseStream(onPreferred eventLoop: EventLoop) -> Connection? {
427+
mutating func leaseStream(onPreferred eventLoop: EventLoop) -> (Connection, LeasedStreamContext)? {
428428
guard let index = self.findAvailableConnection(onPreferred: eventLoop) else { return nil }
429429
return self.leaseStreams(at: index, count: 1)
430430
}
@@ -443,7 +443,7 @@ extension HTTPConnectionPool {
443443
return availableConnectionIndex
444444
}
445445

446-
mutating func leaseStream(onRequired eventLoop: EventLoop) -> Connection? {
446+
mutating func leaseStream(onRequired eventLoop: EventLoop) -> (Connection, LeasedStreamContext)? {
447447
guard let index = self.findAvailableConnection(onRequired: eventLoop) else { return nil }
448448
return self.leaseStreams(at: index, count: 1)
449449
}
@@ -459,8 +459,11 @@ extension HTTPConnectionPool {
459459
/// - count: number of streams you want to lease. You get the current available streams from the `AvailableConnectionContext` which `newHTTP2ConnectionEstablished(_:maxConcurrentStreams:)` returns
460460
/// - Returns: connection to execute `count` requests on
461461
/// - precondition: `index` needs to be valid. `count` must be greater than or equal to *0* and not execeed the number of available streams.
462-
mutating func leaseStreams(at index: Int, count: Int) -> Connection {
463-
self.connections[index].lease(count)
462+
mutating func leaseStreams(at index: Int, count: Int) -> (Connection, LeasedStreamContext) {
463+
let isIdle = self.connections[index].isIdle
464+
let connection = self.connections[index].lease(count)
465+
let context = LeasedStreamContext(wasIdle: isIdle)
466+
return (connection, context)
464467
}
465468

466469
mutating func releaseStream(_ connectionID: Connection.ID) -> (Int, AvailableConnectionContext) {
@@ -559,6 +562,11 @@ extension HTTPConnectionPool {
559562
var isIdle: Bool
560563
}
561564

565+
struct LeasedStreamContext {
566+
/// true if the connection was idle before leasing the stream
567+
var wasIdle: Bool
568+
}
569+
562570
struct GoAwayContext {
563571
/// The eventLoop the connection is running on.
564572
var eventLoop: EventLoop

‎Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP2ConnectionsTest.swift‎

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
3636
XCTAssertEqual(conn1CreatedContext.availableStreams, 100)
3737
XCTAssertEqual(conn1CreatedContext.isIdle, true)
3838
XCTAssert(conn1CreatedContext.eventLoop === el1)
39-
XCTAssertEqual(connections.leaseStreams(at: conn1Index, count: 1), conn1)
39+
let (leasedConn1, leasdConnContext1) = connections.leaseStreams(at: conn1Index, count: 1)
40+
XCTAssertEqual(leasedConn1, conn1)
41+
XCTAssertEqual(leasdConnContext1.wasIdle, true)
4042

4143
// eventLoop connection
4244
XCTAssertTrue(connections.hasConnectionThatCanOrWillBeAbleToExecuteRequests)
@@ -48,7 +50,10 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
4850
XCTAssertEqual(conn1CreatedContext.availableStreams, 100)
4951
XCTAssertTrue(conn1CreatedContext.isIdle)
5052
XCTAssert(conn2CreatedContext.eventLoop === el2)
51-
XCTAssertEqual(connections.leaseStreams(at: conn2Index, count: 1), conn2)
53+
54+
let (leasedConn2, leasdConnContext2) = connections.leaseStreams(at: conn2Index, count: 1)
55+
XCTAssertEqual(leasedConn2, conn2)
56+
XCTAssertEqual(leasdConnContext2.wasIdle, true)
5257
XCTAssertTrue(connections.hasConnectionThatCanOrWillBeAbleToExecuteRequests(for: el2))
5358
}
5459

@@ -184,7 +189,10 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
184189
let conn2ID = connections.createNewConnection(on: el1)
185190
let conn2: HTTPConnectionPool.Connection = .__testOnly_connection(id: conn2ID, eventLoop: el1)
186191
let (conn2Index, _) = connections.newHTTP2ConnectionEstablished(conn2, maxConcurrentStreams: 100)
187-
XCTAssertEqual(connections.leaseStreams(at: conn2Index, count: 1), conn2)
192+
193+
let (leasedConn1, leasdConnContext1) = connections.leaseStreams(at: conn2Index, count: 1)
194+
XCTAssertEqual(leasedConn1, conn2)
195+
XCTAssertEqual(leasdConnContext1.wasIdle, true)
188196
XCTAssertNil(connections.closeConnectionIfIdle(conn2ID))
189197
}
190198

@@ -201,8 +209,11 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
201209
_ = connections.newHTTP2ConnectionEstablished(conn1, maxConcurrentStreams: 100)
202210

203211
// connection is leased
204-
let lease = connections.leaseStream(onPreferred: el1)
205-
XCTAssertEqual(lease, conn1)
212+
guard let (leasedConn, leaseContext) = connections.leaseStream(onPreferred: el1) else {
213+
return XCTFail("lease unexpectedly failed")
214+
}
215+
XCTAssertEqual(leasedConn, conn1)
216+
XCTAssertEqual(leaseContext.wasIdle, true)
206217

207218
// timeout arrives minimal to late
208219
XCTAssertEqual(connections.closeConnectionIfIdle(conn1ID), nil)
@@ -238,7 +249,11 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
238249
_ = connections.newHTTP2ConnectionEstablished(conn1, maxConcurrentStreams: 100)
239250

240251
// we lease it just before timeout
241-
XCTAssertEqual(connections.leaseStream(onRequired: el1), conn1)
252+
guard let (leasedConn, leaseContext) = connections.leaseStream(onRequired: el1) else {
253+
return XCTFail("lease unexpectedly failed")
254+
}
255+
XCTAssertEqual(leasedConn, conn1)
256+
XCTAssertEqual(leaseContext.wasIdle, true)
242257

243258
// timeout arrives minimal to late
244259
XCTAssertEqual(connections.closeConnectionIfIdle(conn1ID), nil)
@@ -273,10 +288,11 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
273288
XCTAssertEqual(connections.stats.idleConnections, 4)
274289

275290
// connection is leased
276-
guard let lease = connections.leaseStream(onPreferred: el1) else {
291+
guard let (leasedConn, leaseContext) = connections.leaseStream(onPreferred: el1) else {
277292
return XCTFail("Expected to be able to lease a connection")
278293
}
279-
XCTAssertEqual(lease, .__testOnly_connection(id: 0, eventLoop: el1))
294+
XCTAssertEqual(leasedConn, .__testOnly_connection(id: 0, eventLoop: el1))
295+
XCTAssertEqual(leaseContext.wasIdle, true)
280296

281297
XCTAssertEqual(connections.stats.backingOffConnections, 0)
282298
XCTAssertEqual(connections.stats.leasedStreams, 1)
@@ -292,7 +308,7 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
292308

293309
let context = connections.shutdown()
294310
XCTAssertEqual(context.close.count, 3)
295-
XCTAssertEqual(context.cancel, [lease])
311+
XCTAssertEqual(context.cancel, [leasedConn])
296312
XCTAssertEqual(context.connectBackoff, [backingOffID])
297313

298314
XCTAssertEqual(connections.stats.idleConnections, 0)
@@ -302,8 +318,8 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
302318
XCTAssertEqual(connections.stats.startingConnections, 1)
303319
XCTAssertFalse(connections.isEmpty)
304320

305-
let (releaseIndex, _) = connections.releaseStream(lease.id)
306-
XCTAssertEqual(connections.closeConnection(at: releaseIndex), lease)
321+
let (releaseIndex, _) = connections.releaseStream(leasedConn.id)
322+
XCTAssertEqual(connections.closeConnection(at: releaseIndex), leasedConn)
307323
XCTAssertFalse(connections.isEmpty)
308324

309325
guard let (failIndex, _) = connections.failConnection(startingID) else {
@@ -322,15 +338,22 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
322338
let conn1: HTTPConnectionPool.Connection = .__testOnly_connection(id: conn1ID, eventLoop: el1)
323339
let (conn1Index, conn1CreatedContext) = connections.newHTTP2ConnectionEstablished(conn1, maxConcurrentStreams: 100)
324340
XCTAssertEqual(conn1CreatedContext.availableStreams, 100)
325-
XCTAssertEqual(connections.leaseStreams(at: conn1Index, count: 100), conn1)
341+
let (leasedConn1, leasdConnContext1) = connections.leaseStreams(at: conn1Index, count: 100)
342+
XCTAssertEqual(leasedConn1, conn1)
343+
XCTAssertEqual(leasdConnContext1.wasIdle, true)
326344

327345
XCTAssertNil(connections.leaseStream(onRequired: el1), "should not be able to lease stream because they are all already leased")
328346

329347
let (_, releaseContext) = connections.releaseStream(conn1ID)
330348
XCTAssertFalse(releaseContext.isIdle)
331349
XCTAssertEqual(releaseContext.availableStreams, 1)
332350

333-
XCTAssertEqual(connections.leaseStream(onRequired: el1), conn1)
351+
guard let (leasedConn, leaseContext) = connections.leaseStream(onRequired: el1) else {
352+
return XCTFail("lease unexpectedly failed")
353+
}
354+
XCTAssertEqual(leasedConn, conn1)
355+
XCTAssertEqual(leaseContext.wasIdle, false)
356+
334357
XCTAssertNil(connections.leaseStream(onRequired: el1), "should not be able to lease stream because they are all already leased")
335358
}
336359

@@ -343,7 +366,10 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
343366
let conn1: HTTPConnectionPool.Connection = .__testOnly_connection(id: conn1ID, eventLoop: el1)
344367
let (conn1Index, conn1CreatedContext) = connections.newHTTP2ConnectionEstablished(conn1, maxConcurrentStreams: 10)
345368
XCTAssertEqual(conn1CreatedContext.availableStreams, 10)
346-
XCTAssertEqual(connections.leaseStreams(at: conn1Index, count: 2), conn1)
369+
370+
let (leasedConn1, leasdConnContext1) = connections.leaseStreams(at: conn1Index, count: 2)
371+
XCTAssertEqual(leasedConn1, conn1)
372+
XCTAssertEqual(leasdConnContext1.wasIdle, true)
347373

348374
XCTAssertTrue(connections.goAwayReceived(conn1ID).eventLoop === el1)
349375

@@ -421,7 +447,10 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
421447
let conn1: HTTPConnectionPool.Connection = .__testOnly_connection(id: conn1ID, eventLoop: el1)
422448
let (conn1Index, conn1CreatedContext) = connections.newHTTP2ConnectionEstablished(conn1, maxConcurrentStreams: 1)
423449
XCTAssertEqual(conn1CreatedContext.availableStreams, 1)
424-
XCTAssertEqual(connections.leaseStreams(at: conn1Index, count: 1), conn1)
450+
451+
let (leasedConn1, leasdConnContext1) = connections.leaseStreams(at: conn1Index, count: 1)
452+
XCTAssertEqual(leasedConn1, conn1)
453+
XCTAssertEqual(leasdConnContext1.wasIdle, true)
425454

426455
XCTAssertNil(connections.leaseStream(onRequired: el1), "all streams are in use")
427456

@@ -430,7 +459,11 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
430459
XCTAssertTrue(newSettingsContext1.eventLoop === el1)
431460
XCTAssertFalse(newSettingsContext1.isIdle)
432461

433-
XCTAssertEqual(connections.leaseStream(onRequired: el1), conn1)
462+
guard let (leasedConn2, leaseContext2) = connections.leaseStream(onRequired: el1) else {
463+
return XCTFail("lease unexpectedly failed")
464+
}
465+
XCTAssertEqual(leasedConn2, conn1)
466+
XCTAssertEqual(leaseContext2.wasIdle, false)
434467

435468
let (_, newSettingsContext2) = connections.newHTTP2MaxConcurrentStreamsReceived(conn1ID, newMaxStreams: 1)
436469
XCTAssertEqual(newSettingsContext2.availableStreams, 0)
@@ -449,7 +482,11 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
449482
XCTAssertTrue(release2Context.isIdle)
450483
XCTAssertEqual(release2Context.availableStreams, 1)
451484

452-
XCTAssertEqual(connections.leaseStream(onRequired: el1), conn1)
485+
guard let (leasedConn3, leaseContext3) = connections.leaseStream(onRequired: el1) else {
486+
return XCTFail("lease unexpectedly failed")
487+
}
488+
XCTAssertEqual(leasedConn3, conn1)
489+
XCTAssertEqual(leaseContext3.wasIdle, true)
453490
}
454491

455492
func testLeaseOnPreferredEventLoopWithoutAnyAvailable() {
@@ -461,7 +498,9 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
461498
let conn1: HTTPConnectionPool.Connection = .__testOnly_connection(id: conn1ID, eventLoop: el1)
462499
let (conn1Index, conn1CreatedContext) = connections.newHTTP2ConnectionEstablished(conn1, maxConcurrentStreams: 1)
463500
XCTAssertEqual(conn1CreatedContext.availableStreams, 1)
464-
XCTAssertEqual(connections.leaseStreams(at: conn1Index, count: 1), conn1)
501+
let (leasedConn1, leasdConnContext1) = connections.leaseStreams(at: conn1Index, count: 1)
502+
XCTAssertEqual(leasedConn1, conn1)
503+
XCTAssertEqual(leasdConnContext1.wasIdle, true)
465504

466505
XCTAssertNil(connections.leaseStream(onPreferred: el1), "all streams are in use")
467506
}
@@ -494,7 +533,11 @@ class HTTPConnectionPool_HTTP2ConnectionsTests: XCTestCase {
494533
let conn1: HTTPConnectionPool.Connection = .__testOnly_connection(id: conn1ID, eventLoop: el1)
495534
let (conn1Index, conn1CreatedContext) = connections.newHTTP2ConnectionEstablished(conn1, maxConcurrentStreams: 100)
496535
XCTAssertEqual(conn1CreatedContext.availableStreams, 100)
497-
XCTAssertEqual(connections.leaseStreams(at: conn1Index, count: 2), conn1)
536+
537+
let (leasedConn1, leasdConnContext1) = connections.leaseStreams(at: conn1Index, count: 2)
538+
XCTAssertEqual(leasedConn1, conn1)
539+
XCTAssertEqual(leasdConnContext1.wasIdle, true)
540+
498541
XCTAssertEqual(
499542
connections.stats,
500543
.init(

0 commit comments

Comments
 (0)