rabbitmq / rabbitmq-objc-client

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

RMQSuspendResumeDispatcher.commandQueue never resumes in some cases when connection goes down #112

Closed vberezkin closed 6 years ago

vberezkin commented 7 years ago
let conn = RMQConnection(delegate: self)
conn.start()
let ch = conn.createChannel()
let exchange = ch.fanout(config.exchange, options: opt)        
conn.blockingClose()

We have a dead lock in conn.blockingClose() if there is not internet connection

michaelklishin commented 7 years ago

So this problem is known as "what to do with continuations in progress when connection is down."

There are two known solutions in other clients:

This client is modelled after Bunny but both options work well and can be considered.

michaelklishin commented 7 years ago

An orthogonal but related improvement would be immediately throwing an exception if connection is in closed state.

michaelklishin commented 7 years ago

@vberezkin so is there any evidence that it has anything to do with RMQConnection#createChannel, or RMQConnection#blockingClose would block anyway, as the name suggests?

michaelklishin commented 7 years ago

@camelpunch do you have an opinion on which options of the above we should pursue?

camelpunch commented 7 years ago

I believe there's no built-in way of cancelling libdispatch operations (unless you use an NSOperationQueue - a big change). I think a timeout might be the way to go, perhaps wrapping every incoming operation in RMQGCDSerialQueue#blockingEnqueue in a block that cancels after a timeout.

Throwing an exception doesn't work well with iOS and isn't something that developers expect to have to handle. An error pointer could be passed in and set, but you still need a way to escape from the block that's running, and you'd also need to thread that needle through several layers. I'd be tempted to just ignore connection problems at the queue abstraction layer and let other app code detect the disconnect.

Of course, we'd want to have a failing test case for this situation before we could be sure our solution worked. I'd want to prove that 'pulling the cord' does indeed cause a deadlock with a reliable automated test.

vberezkin commented 7 years ago

https://github.com/rabbitmq/rabbitmq-objc-client/pull/113

joshrl commented 7 years ago

One thing to add to this: we have observed that even if you use the non-blocking close function, the thread is still deadlocked and never gets cleaned up.

michaelklishin commented 6 years ago

Should be addressed by #137. Thanks, @tonyli508!