PROTON-2843: [Go] Fix segfault with acknowledging a message on closed receiver

[PatrickTaibel]

Deliveries can not be settled after the receiver has been freed. The current Go implementation leads to a segmentation fault when the receiver is closed before a delivery is acknowledged.

Therefore, this adds a check to ensure that the receiver was not closed (which ensures that Free has not been called on the link yet). Additionally, this changes to use InjectWait in order to be able to use the newly returned error.

[astitcher]

This looks like it fixes the segv, but it seems to change the async nature of the acknowledgement which might be important in some applications for performance. It is certainly true that acknowledge's signature can return an error (and it currently doesn't) so that is an improvement in behaviour. I'm absolutely not very experienced with golang but it looks to me that the Inject implementation doesn't handle the case of the connection being done which is the fundemental issue here. Perhaps Inject needs to wrap the injected function similarly to InjectWait to make sure that the run queue is not done before running the injected function. This would not allow for an error return in this case, but seems to me more consonant with the existing behaviour. Maybe there needs to be a way to return this kind of asynch error (but maybe that's a much bigger API change). @PatrickTaibel wdyt?

[PatrickTaibel]

This looks like it fixes the segv, but it seems to change the async nature of the acknowledgement which might be important in some applications for performance.

Inject itself is not even fully async as it is backed by an unbuffered channel (any previous insertion with Inject will block this call). But I do agree that making this call completely sync is not ideal as we don't need waiting for the actual settle call. I have rewritten this change to use Inject but wait for the confirmation that the receiver can be used. As Error can be used as an indicator for the receiver status, I now use it directly. Additionally, I have added a small test.

I'm absolutely not very experienced with golang but it looks to me that the Inject implementation doesn't handle the case of the connection being done which is the fundemental issue here.

If you mean the whole AMQP connection, that one is guarded by the running channel on the Engine. When the connection closes the Engine closes too and Inject returns Closed as error object. The problem with the segv is the closing of the receiver which can happen independently of the connection.

It is certainly true that acknowledge's signature can return an error (and it currently doesn't) so that is an improvement in behaviour.

Just to avoid confusion: That is not true. In case the connection closes / Engine shuts down this will return an error. Just in case the receiver closes it won't.