Deadlock when LDAP server sends unexpected response packet

[jlhawn]

I have encountered an issue where a Bind Request to a particular LDAP server response with more than one response packet for the corresponding request messageID. The Bind() method only expects a single response which it reads and then returns (running a deferred l.finishMessage(messageID)).

In the meantime, the processMessages() goroutine is blocking on sending this unexpected extra response packet to the result channel for the Bind request, but the Bind() method never receives.

Going back to the deferred l.finishMessage(messageID): The Bind() method wont return until this deferred call finishes but it is blocked on sending the MessageFinish message packet to the processMessages() goroutine which in turn is blocked because the Bind() method never receives from its channel. This is the deadlock.

It seems that this design for the proccesMessages() goroutine would, in general, always end up in deadlock if the server responds with more request packets than a request handler is expecting. This may be in violation of the protocol but I think that the client should be more resilient than this in handling a misbehaving server.

One solution that I've come up with is to make it so that calls to finishMessage() also signal to the processMessages() goroutine to stop blocking on an attempt to send a response packet for that message ID. Currently, the connection has a chanResults map which holds results channels which correspond to specific message IDs. We could replace this with a mapping of message IDs to context objects for that message. At the very least, this context would contain 2 items:

type messageContext struct {
    done      chan struct{}
    responses chan *PacketResponse
}

The job of finishMessage(messageID) would change to acquiring a lock on this message context mapping, closing the done channel, then send the MessageFinish packet. In processMessages() we will have to change the MessageResponse handler to have a select statement which attempts to either send the response packet or receive from the done channel. If this done channel is closed it will not block and return instantly. The value is not important, but this signals that the request handler is done and we can abandon sending response packets to it.

[jlhawn]

Please see #69 for a patch which implements this.