Repository: apple/swift-nio
File: Sources/NIOPosix/BaseStreamSocketChannel.swift (Swift)

Code before:
                // Gathering write
                try self.socket.writev(iovecs: ptrs)
            },
            scalarFileWriteOperation: { descriptor, index, endIndex in
                try self.socket.sendFile(fd: descriptor, offset: index, count: endIndex - index)
            }
        )
        return result
    }

    final override func close0(error: Error, mode: CloseMode, promise: EventLoopPromise<Void>?) {
        do {
            switch mode {
            case .output:
                if self.outputShutdown {
                    promise?.fail(ChannelError._outputClosed)
                    return
                }
                if self.inputShutdown {
                    // Escalate to full closure
                    self.close0(error: error, mode: .all, promise: promise)
                    return
                }
                self.unregisterForWritable()

                let writesCloseResult = self.pendingWrites.closeOutbound(promise)
                switch writesCloseResult {
                case .pending:
                    ()  // promise is stored in `pendingWrites` state for completing on later call to `closeComplete`
                case .readyForClose(let closePromise):
                    // Shutdown the socket only when the pending writes are dealt with
                    do {
                        try self.shutdownSocket(mode: mode)
                        self.pendingWrites.closeComplete()
                    } catch let err {
                        self.pendingWrites.closeComplete(err)
                    }
                    closePromise?.succeed(())
                    self.pipeline.fireUserInboundEventTriggered(ChannelEvent.outputClosed)
                case .closed(let closePromise):
                    closePromise?.succeed(())
                case .open:
                    promise?.fail(ChannelError.inappropriateOperationForState)
                    assertionFailure("Close resulted in an open state, this should never happen")
                }

            case .input:
                if self.inputShutdown {
                    promise?.fail(ChannelError._inputClosed)
                    return
                }

Review comment (bug):
Ah, ok: there's a test missing here. The `unregisterForWritable` above isn't appropriate. I believe the following situation is possible:

1. Many writes are done, such that we are no longer writable at the socket layer.
2. The user issues a final flush.
3. The user calls close(mode: .output)

In this case, we'll have flushed, unwritten data, but we will no longer be registered for writable. As long as no other flush or close comes along, we'll stay unregistered for writable, and so we will make no attempt to empty the buffer. That will cause us to be wedged open.

I recommend, before fixing the bug, you write a test that correctly reproduces this wedge, and use that to validate the bug is actually fixed. This is going to be a challenging test to write. It _can_ be written using both regular sockets, but it can also be written using the SAL, which may be easier. Let me know if you'd like to pair on a test for this.

Code after:
                // Gathering write
                try self.socket.writev(iovecs: ptrs)
            },
            scalarFileWriteOperation: { descriptor, index, endIndex in
                try self.socket.sendFile(fd: descriptor, offset: index, count: endIndex - index)
            }
        )
        return result
    }

    final override func close0(error: Error, mode: CloseMode, promise: EventLoopPromise<Void>?) {
        do {
            switch mode {
            case .output:
                if self.outputShutdown {
                    promise?.fail(ChannelError._outputClosed)
                    return
                }
                if self.inputShutdown {
                    // Escalate to full closure
                    self.close0(error: error, mode: .all, promise: promise)
                    return
                }

                let result = self.pendingWrites.closeOutbound(promise)
                switch result {
                case .pending:
                    ()  // promise is stored in `pendingWrites` state for completing later

                case .readyForClose(let closePromise):
                    // Shutdown the socket only when the pending writes are dealt with
                    do {
                        try self.shutdownSocket(mode: mode)
                        closePromise?.succeed(())
                    } catch let err {
                        closePromise?.fail(err)
                    }
                    self.unregisterForWritable()
                    self.pipeline.fireUserInboundEventTriggered(ChannelEvent.outputClosed)

                case .closed(let closePromise):
                    closePromise?.succeed(())

                case .errored(let err, let closePromise):
                    assertionFailure("Close errored: \(err)")
                    closePromise?.fail(err)

                    // Escalate to full closure
                    // promise is nil here because we have used the supplied promise to convey failure of the half-close
                    self.close0(error: err, mode: .all, promise: nil)
                }