PatrickTaibel commented on PR #432: URL: https://github.com/apache/qpid-proton/pull/432#issuecomment-2370969825
> 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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
