search

vapor / websocket-kit

WebSocket client library built on SwiftNIO
https://docs.vapor.codes/4.0/advanced/websockets/
MIT License
272 stars 79 forks source link

Updated WebSocketClient to use ManagedAtomic instead of NIO. #122

Closed JetForMe closed 2 years ago

JetForMe commented 2 years ago

Fixes #121

Ordering is based on the ordering specified in the comments of NIOAtomic compareAndExchange() and load().

Replaced the sole instance of NIOAtomic.makeAtomic(value:) with ManagedAtomic<Bool>(_:), as well as the two accessed methods compareAndExchange() and load(). The new atomics require specifying the memory ordering. Based on the comments in NIOConcurrencyHelpers, the compare-and-exchange is done with .sequentiallyConsistent ordering, and the load is done with .relaxed.

0xTim commented 2 years ago

Superseded by #120

0xTim commented 2 years ago

(Just seeing if CI failures are universal)

codecov-commenter commented 2 years ago

Codecov Report

Merging #122 (3669bdb) into main (efb1bd5) will decrease coverage by 6.95%. The diff coverage is 33.33%.

@@            Coverage Diff             @@
##             main     #122      +/-   ##
==========================================
- Coverage   73.95%   67.00%   -6.96%     
==========================================
  Files           5        6       +1     
  Lines         503      597      +94     
==========================================
+ Hits          372      400      +28     
- Misses        131      197      +66
Flag Coverage Δ
unittests 67.00% <33.33%> (-6.96%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
Sources/WebSocketKit/WebSocketClient.swift 79.33% <33.33%> (ø)
...bSocketKit/Concurrency/WebSocket+Concurrency.swift 29.78% <0.00%> (ø)
JetForMe commented 2 years ago

Superseded by #120

Derp. That's what I get for leaving is:issue in when I searched. Honestly I wish Github wouldn't default to any constraints.