NIOLock and NIOLockedValueBox are too strict

[Lukasa]

NIOLock is particularly scary, but NIOLockedValueBox has some limitations too that need to be tidied up.

The following functions are dangerous:

swift-nio/Sources/NIOConcurrencyHelpers/NIOLock.swift

Lines 232 to 254 in 9ff5fdd

	extension NIOLock {
	/// Acquire the lock for the duration of the given block.
	///
	/// This convenience method should be preferred to `lock` and `unlock` in
	/// most situations, as it ensures that the lock will be released regardless
	/// of how `body` exits.
	///
	/// - Parameter body: The block to execute while holding the lock.
	/// - Returns: The value returned by the block.
	@inlinable
	public func withLock<T>(_ body: () throws -> T) rethrows -> T {
	self.lock()
	defer {
	self.unlock()
	}
	return try body()
	}
	@inlinable
	public func withLockVoid(_ body: () throws -> Void) rethrows {
	try self.withLock(body)
	}
	}

Here, withLock needs to take its closure sending and the return value needs to be sending too. In Swift 5, these both need to be Sendable, with an unchecked version available.

swift-nio/Sources/NIOConcurrencyHelpers/NIOLockedValueBox.swift

Lines 35 to 39 in 9ff5fdd

	/// Access the `Value`, allowing mutation of it.
	@inlinable
	public func withLockedValue<T>(_ mutate: (inout Value) throws -> T) rethrows -> T {
	try self._storage.withLockedValue(mutate)
	}

Here, withLockedValue needs again to take its closure sending. Optionally, we could relax its Sendable requirement, and also return the value sending, but given our need to remain compatible with Swift 5 we should probably leave it stricter for now. In Swift 5, the closure probably needs to be @Sendable.

[FranzBusch]

Yep good catch. We need to basically copy the API from Mutex on the LockedValueBox here

[weissi]

@Lukasa Why does NIOLock.withLock need sending? It doesn't contain a value (so there's nothing to escape) and it never changes isolation or anything (no async etc).

And NIOLockedValueBox is currently only Sendable iff Value: Sendable, so that seems fine too as it can be escaped safely.

---

Said all that, it might be useful for developers if they were sending (just in case they want to pass an isolated closure). But I don't see a not-strict-enough issue here.

[weissi]

I think we should reverb this ticket as it's currently written confusingly. My summary would be:

• Most importantly: NIOLockedValueBox as well as NIOLock are correct regarding sendability etc
• NIOLock should stay is as and should not get a sending in withLock (as that would make it less useful)
• NIOLockedValueBox is more strict than it needs to be as of Swift 6.0. It could be even more helpful if it took sending closures as that would allow us to make it unconditionally Sendable.

[weissi]

@Lukasa can we close this bug or retitle to "NIOLockedValueBox too strict"?