Data race fix

[SimplyKyra]

Was getting a data race issue when I started importing images on multiple threads.

The error was NIO-ELT-0-#0 (12): Assertion failed: Attempt to send request with request ID 35 already in flight.. The code errored in SFTPClient within SendRequest on line 84 assert for self.responses.responses[requestId] == nil.

I made sure the nextRequestId was incremented on the main actor thread so we didn't get duplicate request IDs.

[Joannis]

Sorry for missing this PR!

[Joannis]

I think we should steer away from using @MainActor here. I'd either like another (our own) globalActor, or preferrably to wrap the value in a NIOLockedValueBox

[Joannis]

If you're unable to make that change, I can work on it sometime later

[Joannis]

Thanks for the PR! I really appreciate the contribution

[SimplyKyra]

Steering away from using @MainActor makes sense. I started looking into the NIOLockedValueBox option you mentioned but realized the package used in Citadel doesn't include it. Wasn't sure what your process was to move over code from Apple's Swift-NIO and I don't have the bandwidth to go through all the steps that might be required right now. That said, in case it helps you, figured I'd drop a link to it: Sources/NIOConcurrencyHelpers/NIOLockedValueBox.swift and share the untested (as it can't find NIOLockedValueBox in scope) code I did up this morning.

    /// Access to the locked box to access the nextRequestId variable
    let requestIDAccess = AllocateRequestID()

    // Call it with: requestIDAccess.allocateRequestId()

    /// A locked box to access the nextRequestId variable so it's thread safe using NIOLockedValueBox
    final class AllocateRequestID {
        /// A monotonically increasing counter for gneerating request IDs.
        private var nextRequestId: UInt32 = 0
        let lockedValueBox: NIOLockedValueBox<UInt32>
        
        init() {
            self.lockedValueBox = NIOLockedValueBox(nextRequestId)
        }
        
        /// Returns a unique request ID for use in an SFTP message. Does _not_ register the ID for
        /// a response; that is handled by `sendRequest(_:)`.
        internal func allocateRequestId() -> UInt32 {
            return self.lockedValueBox.withLock { requestId in
                defer {
                    requestId &+= 1
                }
                return requestId
            }
        }
    }

Thanks for making Citadel open. It's been really helpful. Going to use my @MainActor fix for now so no rush fixing this if you don't have the bandwidth either :)