astitcher commented on PR #432: URL: https://github.com/apache/qpid-proton/pull/432#issuecomment-2344213433
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? -- 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
