Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make NIOLoopBoundBox off-EL with sending #3091

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions Sources/NIOCore/NIOLoopBound.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
///
/// ``NIOLoopBound`` is useful to transport a value of a non-`Sendable` type that needs to go from one place in
/// your code to another where you (but not the compiler) know is on one and the same ``EventLoop``. Usually this
/// involves `@Sendable` closures. This type is safe because it verifies (using ``EventLoop/preconditionInEventLoop(file:line:)-2fxvb``)
/// involves `@Sendable` or `sending` closures. This type is safe because it verifies (using ``EventLoop/preconditionInEventLoop(file:line:)-2fxvb``)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but thought this would be more precise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related: should I add more explicit documentation outlining that there are a bunch of functions to make a NIOLoopBoundBox off the event loop called make*, and outline the use cases for them at a high level? Separate PR?

/// that this is actually true.
///
/// A ``NIOLoopBound`` can only be constructed, read from or written to when you are provably
Expand Down Expand Up @@ -59,7 +59,7 @@ public struct NIOLoopBound<Value>: @unchecked Sendable {
///
/// ``NIOLoopBoundBox`` is useful to transport a value of a non-`Sendable` type that needs to go from one place in
/// your code to another where you (but not the compiler) know is on one and the same ``EventLoop``. Usually this
/// involves `@Sendable` closures. This type is safe because it verifies (using ``EventLoop/preconditionInEventLoop(file:line:)-7ukrq``)
/// involves `@Sendable` or `sending` closures. This type is safe because it verifies (using ``EventLoop/preconditionInEventLoop(file:line:)-7ukrq``)
/// that this is actually true.
///
/// A ``NIOLoopBoundBox`` can only be read from or written to when you are provably
Expand Down Expand Up @@ -142,3 +142,26 @@ public final class NIOLoopBoundBox<Value>: @unchecked Sendable {
}
}
}

#if compiler(>=6.0) // `sending` is >= 6.0
extension NIOLoopBoundBox {
/// Initialise a ``NIOLoopBoundBox`` by `sending` (i.e. transferring) a value, validly callable off `eventLoop`.
///
/// Contrary to ``init(_:eventLoop:)``, this method can be called off `eventLoop` because we are `sending` the value across the isolation domain.
/// Because we're `sending` `value`, we just need to protect it against mutations (because nothing else can have access to it anymore).
public static func makeBoxSendingValue(
_ value: sending Value,
as: Value.Type = Value.self,
eventLoop: EventLoop
) -> NIOLoopBoundBox<Value> {
// Here, we -- possibly surprisingly -- do not precondition being on the EventLoop. This is okay for a few
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is a copy-pasta from the other off-EL initializers, but I think it's fair to keep this for consistency.

// reasons:
// - This function only works by `sending` the value, so we don't need to worry about somebody
// still holding a reference to this — Swift 6 will ensure the value is not modified erroneously.
// - Because of Swift's Definitive Initialisation (DI), we know that we did write `self._value` before `init`
// returns.
// - The only way to ever write (or read indeed) `self._value` is by proving to be inside the `EventLoop`.
.init(_value: value, uncheckedEventLoop: eventLoop)
}
}
#endif
36 changes: 36 additions & 0 deletions Tests/NIOPosixTests/NIOLoopBoundTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,42 @@ final class NIOLoopBoundTests: XCTestCase {
)
}

#if compiler(>=6.0)
fileprivate class NonSendableThingy {
var value: Int = 0
init(value: Int) {
self.value = value
}
}

func testLoopBoundBoxCanBeInitialisedWithSendingValueOffLoopAndLaterSetToValue() {
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer {
XCTAssertNoThrow(try group.syncShutdownGracefully())
}

let loop = group.any()
let nonSendableThingy = NonSendableThingy(value: 15)

let sendableBox = NIOLoopBoundBox.makeBoxSendingValue(
nonSendableThingy,
as: NonSendableThingy.self,
eventLoop: loop
)
for _ in 0..<(100 - 15) {
loop.execute {
sendableBox.value.value += 1
}
}
XCTAssertEqual(
100,
try loop.submit {
sendableBox.value.value
}.wait()
)
}
#endif

func testInPlaceMutation() {
var loopBound = NIOLoopBound(CoWValue(), eventLoop: loop)
XCTAssertTrue(loopBound.value.mutateInPlace())
Expand Down