Close/deinit should be automatic

[LeifOfWilsonCreek]

First, thank you for writing this. RMQClient was my first try, and is extremely frustrating. 👍

Two bug related to close. First:

deinit {
    if isConnected {
        assertionFailure("close() was not called before deinit!")
    }
}

should be

deinit {
    if isConnected {
        try? await close()
    }
}

because otherwise writing the obvious usage doesn't work and alternatives becomes nigh unreadable:

do {
    let conn = try await AMQPConnection.connect(...)
    let chan = try await conn.openChannel()
    try await chan.basicQos(count: 1000, global: false)
    self.connection = conn
    self.channel = chan
} catch {
    print("Failed \(url); error=\(error)")
}

This doesn't work because if conn.openChannel() fails, conn doesn't get an explicit conn.close(), and AMQPConnection asserts. There's no need to make the caller jump through hoops calling .close() explicitly.

Second:

func close(reason: String = "", code: UInt16 = 200) async throws {
    return try await self.close(reason: reason, code: code).get()
}

should be:

func close(reason: String = "", code: UInt16 = 200) async throws {
    if !isConnected {return}  // insert this line
    return try await self.close(reason: reason, code: code).get()
}

Because when the connection is already closed, calling close (a very common mistake when trying to work around the above), causes an infinite hang. Even more confusingly, this hang also happens when an error closes the socket, then the caller tries to call .close() in cleanup.

Those two quick fixes save huge workarounds. Thanks 👍

[funcmike]

Hi, thanks for reporting issues. I need to think of how to handle this properly because await can't be used on deinit { }. Second thing - bug on double close should be relatively easy to fix. I'll do it next week.

[xtremekforever]

Hi, thanks for reporting issues. I need to think of how to handle this properly because await can't be used on deinit { }. Second thing - bug on double close should be relatively easy to fix. I'll do it next week.

You can always try to spin off a Task {} in deinit, but it's ugly.

[LeifOfWilsonCreek]

Hi, thanks for reporting issues. I need to think of how to handle this properly because await can't be used on deinit { }. Second thing - bug on double close should be relatively easy to fix. I'll do it next week.

You can always try to spin off a Task {} in deinit, but it's ugly.

I commonly use this function to await in sync context:

func syncawait<T>(_ operation: sending @escaping () async -> T) -> T where T: Sendable {
    var result: T!
    let group = DispatchGroup()
    group.enter()

    // Detaching is necessary to avoid a deadlock on `group.wait()`.
    // Task.detached resets priority to .medium unless explicitly told.
    // Since the caller is waiting for the Task, changing the priority would not make sense.
    let qos = qos_class_self()
    let priority = TaskPriority(rawValue: UInt8(qos.rawValue))
    Task.detached(priority: priority) {
        result = await operation()
        group.leave()
    }
    group.wait()
    return result
}

then call it with

syncawait {await close()}

I think that's on-point here iiuc. 👍