Skip to content

Commit 56724a2

Browse files
rnrornro
authored
NIOAsyncTestingChannel local/remote addrs on EmbeddedChannelCore (#3442)
### Motivation: `NIOAsyncTestingChannel` stored its `localAddress` and `remoteAddress` in a locked storage on itself for thread safety, however in doing so left us open to bugs because a handler grabbing the addresses of the context had no visibility of the values. ### Modifications: Reach into `EmbeddedChannelCore` for the addresses instead of storing them on the `NIOAsyncTestinghannel`. I also considered a delegate approach where the `EmbeddedChannelCore` could offload the responsibility for storing the values back to the `NIOAsyncTestingChannel` but it was complicated and of questionable value. ### Result: * The correct address values are seen no matter how they are obtained. * We probably take a performance hit locking the values in this way but this is testing code so probably not the end of the world.
1 parent 2dbd1bf commit 56724a2

File tree

11 files changed

+95
-56
lines changed

11 files changed

+95
-56
lines changed

‎Benchmarks/Thresholds/6.0/NIOPosixBenchmarks.TCPEcho.p90.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
{
2-
"mallocCountTotal" : 108
2+
"mallocCountTotal": 108
33
}

‎Benchmarks/Thresholds/6.0/NIOPosixBenchmarks.TCPEchoAsyncChannel.p90.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
{
2-
"mallocCountTotal" : 164376
3-
}
2+
"mallocCountTotal": 165111
3+
}

‎Benchmarks/Thresholds/nightly-main/NIOPosixBenchmarks.TCPEchoAsyncChannel.p90.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
{
2-
"mallocCountTotal": 82000
2+
"mallocCountTotal" : 82672
33
}

‎IntegrationTests/tests_04_performance/Thresholds/6.0.json‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,47 @@
11
{
2-
"10000000_asyncsequenceproducer": 19,
3-
"1000000_asyncwriter": 1000050,
4-
"1000_addHandlers": 44050,
5-
"1000_addHandlers_sync": 37050,
2+
"1_reqs_1000_conn": 375050,
3+
"1000_addHandlers_sync": 38050,
4+
"1000_addHandlers": 45050,
65
"1000_addRemoveHandlers_handlercontext": 8050,
76
"1000_addRemoveHandlers_handlername": 8050,
87
"1000_addRemoveHandlers_handlertype": 8050,
9-
"1000_autoReadGetAndSet": 18050,
108
"1000_autoReadGetAndSet_sync": 0,
9+
"1000_autoReadGetAndSet": 18050,
1110
"1000_copying_bytebufferview_to_array": 1050,
1211
"1000_copying_circularbuffer_to_array": 1050,
12+
"1000_getHandlers_sync": 36,
1313
"1000_getHandlers": 8050,
14-
"1000_getHandlers_sync": 35,
1514
"1000_reqs_1_conn": 26350,
1615
"1000_rst_connections": 138050,
1716
"1000_tcpbootstraps": 3050,
1817
"1000_tcpconnections": 146050,
1918
"1000_udp_reqs": 6050,
2019
"1000_udpbootstraps": 2050,
2120
"1000_udpconnections": 75050,
22-
"1_reqs_1000_conn": 375050,
21+
"1000000_asyncwriter": 1000050,
22+
"10000000_asyncsequenceproducer": 19,
2323
"assume_isolated_scheduling_10000_executions": 89,
2424
"bytebuffer_lots_of_rw": 2050,
2525
"creating_10000_headers": 0,
2626
"decode_1000_ws_frames": 2050,
27-
"encode_1000_ws_frames_holding_buffer": 3,
2827
"encode_1000_ws_frames_holding_buffer_with_mask": 2050,
29-
"encode_1000_ws_frames_holding_buffer_with_space": 3,
3028
"encode_1000_ws_frames_holding_buffer_with_space_with_mask": 2050,
31-
"encode_1000_ws_frames_new_buffer": 3050,
29+
"encode_1000_ws_frames_holding_buffer_with_space": 3,
30+
"encode_1000_ws_frames_holding_buffer": 3,
3231
"encode_1000_ws_frames_new_buffer_with_mask": 5050,
33-
"encode_1000_ws_frames_new_buffer_with_space": 3050,
3432
"encode_1000_ws_frames_new_buffer_with_space_with_mask": 5050,
33+
"encode_1000_ws_frames_new_buffer_with_space": 3050,
34+
"encode_1000_ws_frames_new_buffer": 3050,
3535
"execute_hop_10000_tasks": 0,
3636
"flat_schedule_10000_tasks": 100100,
3737
"flat_schedule_assume_isolated_10000_tasks": 90100,
3838
"future_assume_isolated_lots_of_callbacks": 68050,
3939
"future_erase_result": 4050,
4040
"future_lots_of_callbacks": 68050,
41-
"get_100000_headers_canonical_form": 700050,
42-
"get_100000_headers_canonical_form_trimming_whitespace": 700050,
4341
"get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 700050,
4442
"get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 700050,
43+
"get_100000_headers_canonical_form_trimming_whitespace": 700050,
44+
"get_100000_headers_canonical_form": 700050,
4545
"modifying_1000_circular_buffer_elements": 0,
4646
"modifying_byte_buffer_view": 6050,
4747
"ping_pong_1000_reqs_1_conn": 305,
@@ -54,6 +54,6 @@
5454
"scheduling_10000_executions": 89,
5555
"submit_10000_tasks": 20100,
5656
"submit_assume_isolated_10000_tasks": 20100,
57-
"udp_1000_reqs_1_conn": 6200,
58-
"udp_1_reqs_1000_conn": 162050
57+
"udp_1_reqs_1000_conn": 162050,
58+
"udp_1000_reqs_1_conn": 6200
5959
}

‎IntegrationTests/tests_04_performance/Thresholds/6.1.json‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,47 @@
11
{
2-
"10000000_asyncsequenceproducer": 19,
3-
"1000000_asyncwriter": 1000050,
4-
"1000_addHandlers": 44050,
5-
"1000_addHandlers_sync": 37050,
2+
"1_reqs_1000_conn": 375050,
3+
"1000_addHandlers_sync": 38050,
4+
"1000_addHandlers": 45050,
65
"1000_addRemoveHandlers_handlercontext": 8050,
76
"1000_addRemoveHandlers_handlername": 8050,
87
"1000_addRemoveHandlers_handlertype": 8050,
9-
"1000_autoReadGetAndSet": 18050,
108
"1000_autoReadGetAndSet_sync": 0,
9+
"1000_autoReadGetAndSet": 18050,
1110
"1000_copying_bytebufferview_to_array": 1050,
1211
"1000_copying_circularbuffer_to_array": 1050,
12+
"1000_getHandlers_sync": 36,
1313
"1000_getHandlers": 8050,
14-
"1000_getHandlers_sync": 35,
1514
"1000_reqs_1_conn": 26350,
1615
"1000_rst_connections": 138050,
1716
"1000_tcpbootstraps": 3050,
1817
"1000_tcpconnections": 146050,
1918
"1000_udp_reqs": 6050,
2019
"1000_udpbootstraps": 2050,
2120
"1000_udpconnections": 75050,
22-
"1_reqs_1000_conn": 375050,
21+
"1000000_asyncwriter": 1000050,
22+
"10000000_asyncsequenceproducer": 19,
2323
"assume_isolated_scheduling_10000_executions": 89,
2424
"bytebuffer_lots_of_rw": 2050,
2525
"creating_10000_headers": 0,
2626
"decode_1000_ws_frames": 2050,
27-
"encode_1000_ws_frames_holding_buffer": 3,
2827
"encode_1000_ws_frames_holding_buffer_with_mask": 2050,
29-
"encode_1000_ws_frames_holding_buffer_with_space": 3,
3028
"encode_1000_ws_frames_holding_buffer_with_space_with_mask": 2050,
31-
"encode_1000_ws_frames_new_buffer": 3050,
29+
"encode_1000_ws_frames_holding_buffer_with_space": 3,
30+
"encode_1000_ws_frames_holding_buffer": 3,
3231
"encode_1000_ws_frames_new_buffer_with_mask": 5050,
33-
"encode_1000_ws_frames_new_buffer_with_space": 3050,
3432
"encode_1000_ws_frames_new_buffer_with_space_with_mask": 5050,
33+
"encode_1000_ws_frames_new_buffer_with_space": 3050,
34+
"encode_1000_ws_frames_new_buffer": 3050,
3535
"execute_hop_10000_tasks": 0,
3636
"flat_schedule_10000_tasks": 100100,
3737
"flat_schedule_assume_isolated_10000_tasks": 90100,
3838
"future_assume_isolated_lots_of_callbacks": 68050,
3939
"future_erase_result": 4050,
4040
"future_lots_of_callbacks": 68050,
41-
"get_100000_headers_canonical_form": 700050,
42-
"get_100000_headers_canonical_form_trimming_whitespace": 700050,
4341
"get_100000_headers_canonical_form_trimming_whitespace_from_long_string": 700050,
4442
"get_100000_headers_canonical_form_trimming_whitespace_from_short_string": 700050,
43+
"get_100000_headers_canonical_form_trimming_whitespace": 700050,
44+
"get_100000_headers_canonical_form": 700050,
4545
"modifying_1000_circular_buffer_elements": 0,
4646
"modifying_byte_buffer_view": 6050,
4747
"ping_pong_1000_reqs_1_conn": 305,
@@ -54,6 +54,6 @@
5454
"scheduling_10000_executions": 89,
5555
"submit_10000_tasks": 20100,
5656
"submit_assume_isolated_10000_tasks": 20100,
57-
"udp_1000_reqs_1_conn": 6200,
58-
"udp_1_reqs_1000_conn": 162050
57+
"udp_1_reqs_1000_conn": 162050,
58+
"udp_1000_reqs_1_conn": 6200
5959
}

‎IntegrationTests/tests_04_performance/Thresholds/6.2.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
{
22
"1_reqs_1000_conn": 375050,
3-
"1000_addHandlers_sync": 37050,
4-
"1000_addHandlers": 44050,
3+
"1000_addHandlers_sync": 38050,
4+
"1000_addHandlers": 45050,
55
"1000_addRemoveHandlers_handlercontext": 8050,
66
"1000_addRemoveHandlers_handlername": 8050,
77
"1000_addRemoveHandlers_handlertype": 8050,
88
"1000_autoReadGetAndSet_sync": 0,
99
"1000_autoReadGetAndSet": 18050,
1010
"1000_copying_bytebufferview_to_array": 1050,
1111
"1000_copying_circularbuffer_to_array": 1050,
12-
"1000_getHandlers_sync": 35,
12+
"1000_getHandlers_sync": 36,
1313
"1000_getHandlers": 8050,
1414
"1000_reqs_1_conn": 26350,
1515
"1000_rst_connections": 138050,

‎IntegrationTests/tests_04_performance/Thresholds/nightly-main.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
{
22
"1_reqs_1000_conn": 375050,
3-
"1000_addHandlers_sync": 37050,
4-
"1000_addHandlers": 44050,
3+
"1000_addHandlers_sync": 38050,
4+
"1000_addHandlers": 45050,
55
"1000_addRemoveHandlers_handlercontext": 8050,
66
"1000_addRemoveHandlers_handlername": 8050,
77
"1000_addRemoveHandlers_handlertype": 8050,
88
"1000_autoReadGetAndSet_sync": 0,
99
"1000_autoReadGetAndSet": 18050,
1010
"1000_copying_bytebufferview_to_array": 1050,
1111
"1000_copying_circularbuffer_to_array": 1050,
12-
"1000_getHandlers_sync": 35,
12+
"1000_getHandlers_sync": 36,
1313
"1000_getHandlers": 8050,
1414
"1000_reqs_1_conn": 26350,
1515
"1000_rst_connections": 138050,

‎IntegrationTests/tests_04_performance/Thresholds/nightly-next.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
{
22
"1_reqs_1000_conn": 375050,
3-
"1000_addHandlers_sync": 37050,
4-
"1000_addHandlers": 44050,
3+
"1000_addHandlers_sync": 38050,
4+
"1000_addHandlers": 45050,
55
"1000_addRemoveHandlers_handlercontext": 8050,
66
"1000_addRemoveHandlers_handlername": 8050,
77
"1000_addRemoveHandlers_handlertype": 8050,
88
"1000_autoReadGetAndSet_sync": 0,
99
"1000_autoReadGetAndSet": 18050,
1010
"1000_copying_bytebufferview_to_array": 1050,
1111
"1000_copying_circularbuffer_to_array": 1050,
12-
"1000_getHandlers_sync": 35,
12+
"1000_getHandlers_sync": 36,
1313
"1000_getHandlers": 8050,
1414
"1000_reqs_1_conn": 26350,
1515
"1000_rst_connections": 138050,

‎Sources/NIOEmbedded/AsyncTestingChannel.swift‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -201,15 +201,14 @@ public final class NIOAsyncTestingChannel: Channel {
201201

202202
// These two variables are only written once, from a single thread, and never written again, so they're _technically_ thread-safe. Most methods cannot safely
203203
// be used from multiple threads, but `isActive`, `isOpen`, `eventLoop`, and `closeFuture` can all safely be used from any thread. Just.
204+
// `EmbeddedChannelCore`'s localAddress and remoteAddress fields are protected by a lock so they are safe to access.
204205
@usableFromInline
205206
nonisolated(unsafe) var channelcore: EmbeddedChannelCore!
206207
nonisolated(unsafe) private var _pipeline: ChannelPipeline!
207208

208209
@usableFromInline
209210
internal struct State: Sendable {
210211
var isWritable: Bool
211-
var localAddress: SocketAddress?
212-
var remoteAddress: SocketAddress?
213212

214213
@usableFromInline
215214
var options: [(option: any ChannelOption, value: any Sendable)]
@@ -218,7 +217,7 @@ public final class NIOAsyncTestingChannel: Channel {
218217
/// Guards any of the getters/setters that can be accessed from any thread.
219218
@usableFromInline
220219
internal let _stateLock = NIOLockedValueBox(
221-
State(isWritable: true, localAddress: nil, remoteAddress: nil, options: [])
220+
State(isWritable: true, options: [])
222221
)
223222

224223
/// - see: `Channel._channelCore`
@@ -246,24 +245,20 @@ public final class NIOAsyncTestingChannel: Channel {
246245
/// - see: `Channel.localAddress`
247246
public var localAddress: SocketAddress? {
248247
get {
249-
self._stateLock.withLockedValue { $0.localAddress }
248+
self.channelcore.localAddress
250249
}
251250
set {
252-
self._stateLock.withLockedValue {
253-
$0.localAddress = newValue
254-
}
251+
self.channelcore.localAddress = newValue
255252
}
256253
}
257254

258255
/// - see: `Channel.remoteAddress`
259256
public var remoteAddress: SocketAddress? {
260257
get {
261-
self._stateLock.withLockedValue { $0.remoteAddress }
258+
self.channelcore.remoteAddress
262259
}
263260
set {
264-
self._stateLock.withLockedValue {
265-
$0.remoteAddress = newValue
266-
}
261+
self.channelcore.remoteAddress = newValue
267262
}
268263
}
269264

‎Sources/NIOEmbedded/Embedded.swift‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -506,10 +506,35 @@ class EmbeddedChannelCore: ChannelCore {
506506
var inboundBufferConsumer: Deque<(NIOAny) -> Void> = []
507507

508508
@usableFromInline
509-
var localAddress: SocketAddress?
509+
internal struct Addresses {
510+
var localAddress: SocketAddress?
511+
var remoteAddress: SocketAddress?
512+
}
513+
514+
@usableFromInline
515+
internal let _addresses: NIOLockedValueBox<Addresses> = NIOLockedValueBox(
516+
.init(localAddress: nil, remoteAddress: nil)
517+
)
518+
519+
@usableFromInline
520+
var localAddress: SocketAddress? {
521+
get {
522+
self._addresses.withLockedValue { $0.localAddress }
523+
}
524+
set {
525+
self._addresses.withLockedValue { $0.localAddress = newValue }
526+
}
527+
}
510528

511529
@usableFromInline
512-
var remoteAddress: SocketAddress?
530+
var remoteAddress: SocketAddress? {
531+
get {
532+
self._addresses.withLockedValue { $0.remoteAddress }
533+
}
534+
set {
535+
self._addresses.withLockedValue { $0.remoteAddress = newValue }
536+
}
537+
}
513538

514539
@usableFromInline
515540
func localAddress0() throws -> SocketAddress {

0 commit comments

Comments
 (0)