[
https://issues.apache.org/jira/browse/PROTON-2843?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17884234#comment-17884234
]
ASF GitHub Bot commented on PROTON-2843:
----------------------------------------
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.
> [Go] Acknowledging a message on a closed receiver results in a segmentation
> fault
> ---------------------------------------------------------------------------------
>
> Key: PROTON-2843
> URL: https://issues.apache.org/jira/browse/PROTON-2843
> Project: Qpid Proton
> Issue Type: Bug
> Components: go-binding
> Reporter: Patrick Taibel
> Assignee: Alan Conway
> Priority: Major
>
> Using the {{electron}} package when acknowledging messages ({{{}Accept(){}}},
> {{Reject()}} or {{Release()}} on a {{{}ReceivedMessage{}}}) a segmentation
> fault occurs if the {{Receiver}} was already closed.
> This happens because the underlying link of the receiver gets freed which,
> according to the docs of {{pn_link_free,}} also frees any unsettled
> deliveries.
> As the actual acknowledgement needs to run in the engine goroutine the check
> for an active receiver has to happen inside the {{electron}} package.
> I'd suggest adding a check if the receiver is still alive in the injected
> function in {{{}func (rm *ReceivedMessage) acknowledge(status uint64)
> error{}}}.
> I plan to do a PR to fix this but it will take a couple of weeks until I get
> there as this is a low priority fix on our end.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
