Illusive error on concurrent close.

[DmitriyMV]

This is somewhat strange, but based on this build there is probably data race around sendM mutex and conn.Close(). In theory one goroutine (1) can get me.IsClosed() == false and continue until me.writer.WriteFrame(f). Meanwhile, another (2) goroutine would finally reach the func (me *Connection) shutdown(err *Error), call atomic.StoreInt32(&me.closed, 1) and block any new Close attempts. But (1) goroutine has already passed the "check" stage and actively trying to send data, while (2) is trying to close the socket.

I'm still not sure about this problem, because I was not able to reproduce it nor locally, nor on travis-ci instance. But from source code perspective it can be there.

Solution is also quite simple, but will require additional check me.IsClosed() == false after goroutine got lock from Connection.sendM and add Connection.sendM lock around me.conn.Close(). I also think that this will not result in any performance penalties.

This is subject for discussion of course. I can prepare merge request if needed.

[michaelklishin]

@DmitriyMV there was a couple of relevant PRs merged recently, is this still relevant?

[DmitriyMV]

Yes - look here: https://github.com/streadway/amqp/blob/8eff000/connection.go#L353 https://github.com/streadway/amqp/blob/8eff000/connection.go#L399

[gerhard]

We're thinking that returning a more descriptive NetErr would make more sense. What do you think?

[DmitriyMV]

That would do - but you will need to wrap underlying connection with some sort of mutex.