rabbitmq / rabbitmq-objc-client

RabbitMQ client for Objective-C and Swift
https://rabbitmq.com
Other
242 stars 84 forks source link

SIGABRT triggered by code in allocatedUserChannels #212

Open d6unusual opened 1 year ago

d6unusual commented 1 year ago

Hi! Since updating Xcode supporting of iOS 16, my app crashes almost each day in different cases. Here the stacktrace where you may see it happens in RMQClient:

`Exception Type: EXC_CRASH (SIGABRT) Exception Codes: 0x0000000000000000, 0x0000000000000000 Triggered by Thread: 11

Last Exception Backtrace: 0 CoreFoundation 0x1bedd7a84 __exceptionPreprocess + 160 (NSException.m:202) 1 libobjc.A.dylib 0x1bd229958 objc_exception_throw + 56 (objc-exception.mm:356) 2 CoreFoundation 0x1bef65554 _CFThrowFormattedException + 104 (CFObject.m:2235) 3 CoreFoundation 0x1bede5834 -[NSArrayM removeObjectsInRange:] + 1600 (NSArrayM.m:339) 4 RMQClient 0x105737af0 -[RMQMultipleChannelAllocator allocatedUserChannels] + 84 (RMQMultipleChannelAllocator.m:108) 5 RMQClient 0x10571a694 55-[RMQConnectionRecover recover:channelAllocator:error:]_block_invoke_3 + 64 (RMQConnectionRecover.m:95) 6 libdispatch.dylib 0x1caafd850 _dispatch_call_block_and_release + 24 (init.c:1518) 7 libdispatch.dylib 0x1caafe7c8 _dispatch_client_callout + 16 (object.m:560) 8 libdispatch.dylib 0x1caad9854 _dispatch_lane_serial_drain$VARIANT$armv81 + 604 (inline_internal.h:2632) 9 libdispatch.dylib 0x1caada2e4 _dispatch_lane_invoke$VARIANT$armv81 + 380 (queue.c:3940) 10 libdispatch.dylib 0x1caae4000 _dispatch_workloop_worker_thread + 612 (queue.c:6846) 11 libsystem_pthread.dylib 0x20b57ab50 _pthread_wqthread + 284 (pthread.c:2618) 12 libsystem_pthread.dylib 0x20b57a67c start_wqthread + 8 (:-1)`

Version 0.12, installed using Pods

Xcode allows to show place in code, where the crash happens:

(NSArray *)allocatedUserChannels {
NSMutableArray *userChannels = [self.channels.allValues mutableCopy];
[userChannels removeObjectAtIndex:0];
return [userChannels sortedArrayUsingComparator:^NSComparisonResult(id ch1, id ch2) {
  return ch1.channelNumber.integerValue > ch2.channelNumber.integerValue;
}];
}
michaelklishin commented 1 year ago

I cannot suggest much without seeing how your code uses channels. Sharing channels between threads is not allowed, for example.

My best guess is that this function would be safer if it used an immutable array.

michaelklishin commented 1 year ago

Right, so according to the stack the issue is with removeObjectsInRange:, which hints at concurrent access to a mutable data structure.

d6unusual commented 1 year ago

This crash has appeared in different cases:

d6unusual commented 1 year ago

Here is the code of setting connection: ` init(link: String, queue: String) { id = UUID().uuidString self.queue = queue super.init()

    connection = RMQConnection(uri: link, verifyPeer: false, delegate: self)
}

func connect(_ completion: @escaping SocketCompletion) {
    resultCompletion = completion

    DispatchQueue.main.async {
        self.connection?.start { [weak self] in
            guard let self = self else { return }

            DispatchQueue.main.async {
                self.createChannel(queue: self.queue)
            }
        }
    }
}

private func createChannel(queue: String) {
    guard let connection = self.connection, connection.isOpen() else {
        close()
        return
    }

    let channel: RMQChannel = connection.createChannel()
    let listener: (RMQMessage) -> () = { [weak self] message in
        guard let self = self, let package = SocketPackage(rmqMessage: message) else {
            return
        }
        self.resultCompletion?(.message(package))
    }

    let cancelation: EmptyCompletion = { [weak self] in
        guard let self = self else { return }

        self.close()
    }

    let consumer = channel.basicConsume(queue, options: .noAck, handler: listener)
    consumer.onCancellation(cancelation)

    self.cancelationListener = cancelation
    self.messageListener = listener
    self.consumer = consumer
    self.channel = channel

    finish()
}`
michaelklishin commented 1 year ago

If you run this slightly modified code, say, 100 times, can you reproduce the SIGABRT?

d6unusual commented 1 year ago

Let's try

d6unusual commented 1 year ago

@michaelklishin do you provide "this slightly modified" code?

michaelklishin commented 1 year ago

@d6unusual the only modification that should be necessary is connection closure so that you do not accumulate/leak them.

d6unusual commented 1 year ago

This is my code of connection closure:

`func closeConnection(_ completion: @escaping SuccessCompletion) { messageListener = nil cancelationListener = nil

    guard requestService.hasConnection else {
        completion(false)
        return
    }

    closeConsumers { [weak self] finishConsumer in
        guard let self = self else { return }

        guard finishConsumer, self.requestService.hasConnection else {
            completion(false)
            return
        }

        self.closeChannel { [weak self] finishChannel in
            guard let self = self else { return }

            guard finishChannel, self.requestService.hasConnection else {
                completion(false)
                return
            }
            self.cancelConnection(completion)
        }
    }
}

func closeConsumers(_ completion: @escaping SuccessCompletion) {
    guard let consumer = consumer else {
        completion(true)
        return
    }

    consumer.onDelivery(nil)
    consumer.onCancellation(nil)
    consumer.cancel()

    self.consumer = nil
    DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) {
        completion(true)
    }
}

func closeChannel(_ completion: @escaping SuccessCompletion) {
    guard let channel = channel, channel.isOpen() else {
        self.channel = nil
        completion(true)
        return
    }

    channel.close { [weak self] in
        guard let self = self else { return }

        self.channel = nil
        completion(true)
    }

    DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { [weak self] in
        guard let self = self else { return }

        guard self.channel != nil else { return }

        self.cancelConnection(completion)
    }
}

func cancelConnection(_ completion: @escaping SuccessCompletion) {
    guard let connection = connection else {
        completion(true)
        return
    }
    connection.close()

    DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { [weak self] in
        guard let self = self else { return }

        guard !connection.isOpen() else {
            connection.transport().close()
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { [weak self] in
                guard let self = self else { return }

                guard !connection.isOpen() else {
                    completion(false)
                    return
                }
                self.connection = nil
                completion(true)
            }
            return
        }
        self.connection = nil
        completion(true)
    }
}`
d6unusual commented 1 year ago

This code repeats any times until flag connection.isOpen() is true

d6unusual commented 1 year ago

I made such difficult structure to close connection, because there are no provided methods with completions, such as func connectionClose(_ completion: (Bool) -> ()) which will let me know that connection really closed

d6unusual commented 1 year ago

I see in your code (inside framework) that you use asynchronous operations to close channel, connection, but in interface you only provide synchronous functions such as func close().

michaelklishin commented 1 year ago

@d6unusual you are welcome to submit a PR that adds async closure functions.

michaelklishin commented 1 year ago

This code repeats any times until flag connection.isOpen() is true

In other words, you do not get any crashes with a loop of these operations in a single thread. What if you modify the example to add some concurrency, e.g. via GCD?

d6unusual commented 1 year ago

No, crash occurs anyway, I've just demonstrated you the connection closure.

michaelklishin commented 1 year ago

@d6unusual OK, one last question: it does happen eventually (always within 100 runs or so) or always on the first run?

d6unusual commented 1 year ago

It occurs random, but the case in background more often

Brahe commented 7 months ago

I am having this exact problem - so I was wondering if there is any fix for this?

kurt0615 commented 3 months ago

is this fix ?

michaelklishin commented 3 months ago

This was not fixed and the root cause is unknown. This is open source software, so anyone is welcome to investigate it and submit a PR.

kurt0615 commented 3 months ago

This was not fixed and the root cause is unknown. This is open source software, so anyone is welcome to investigate it and submit a PR.

@michaelklishin there is any client for rabbitmq ?