Re: [PR] PROTON-2792: [cpp] Segmentation fault in container::impl::run_timer_jobs [qpid-proton]
astitcher commented on PR #421: URL: https://github.com/apache/qpid-proton/pull/421#issuecomment-1981610822 There is a clear thread safety violation in ```run_timer_jobs`` use of ```is_active_``` without using the appropriate lock - in fact this is so obvious that I'm puzzled we haven't seen it in the TSAN or even static analyzer runs - maybe we just missed it! Unfortunately this fix has changed the previous semantics of task cancellation: An important way to cancel a task is from a previous task callback. However this change makes that unreliable as it only checks for task cancellation before executing the entire batch of tasks that are runable at the point where ```run_timer_jobs``` is called. So if an earlier task in the batch cancels a later task in the batch then that cancellation would not be honored. I'm looking at a fix that should work correctly now and I'll post it here when I'm done. -- 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
[jira] [Commented] (PROTON-2792) [cpp] Segmentation fault in container::impl::run_timer_jobs
[
https://issues.apache.org/jira/browse/PROTON-2792?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17824146#comment-17824146
]
ASF GitHub Bot commented on PROTON-2792:
astitcher commented on PR #421:
URL: https://github.com/apache/qpid-proton/pull/421#issuecomment-1981610822
There is a clear thread safety violation in ```run_timer_jobs`` use of
```is_active_``` without using the appropriate lock - in fact this is so
obvious that I'm puzzled we haven't seen it in the TSAN or even static analyzer
runs - maybe we just missed it!
Unfortunately this fix has changed the previous semantics of task
cancellation: An important way to cancel a task is from a previous task
callback. However this change makes that unreliable as it only checks for task
cancellation before executing the entire batch of tasks that are runable at the
point where ```run_timer_jobs``` is called. So if an earlier task in the batch
cancels a later task in the batch then that cancellation would not be honored.
I'm looking at a fix that should work correctly now and I'll post it here
when I'm done.
> [cpp] Segmentation fault in container::impl::run_timer_jobs
> ---
>
> Key: PROTON-2792
> URL: https://issues.apache.org/jira/browse/PROTON-2792
> Project: Qpid Proton
> Issue Type: Bug
> Components: cpp-binding
>Affects Versions: proton-c-0.38.0
>Reporter: Martin Zlomek
>Priority: Major
>
> PROTON-2438 introduced a race condition in
> [reading|https://github.com/DreamPearl/qpid-proton/blob/8142e3cecd9f668992e76a5448afc09fd7b1030a/cpp/src/proactor_container_impl.cpp#L545]
> /
> [writing|https://github.com/DreamPearl/qpid-proton/blob/8142e3cecd9f668992e76a5448afc09fd7b1030a/cpp/src/proactor_container_impl.cpp#L547]
> {{is_active_}} in
> [{{run_timer_jobs()}}|https://github.com/DreamPearl/qpid-proton/blob/8142e3cecd9f668992e76a5448afc09fd7b1030a/cpp/src/proactor_container_impl.cpp#L498]
> while modifying it in
> [{{schedule()}}|https://github.com/DreamPearl/qpid-proton/blob/8142e3cecd9f668992e76a5448afc09fd7b1030a/cpp/src/proactor_container_impl.cpp#L455]
> or
> [{{cancel()}}|https://github.com/DreamPearl/qpid-proton/blob/8142e3cecd9f668992e76a5448afc09fd7b1030a/cpp/src/proactor_container_impl.cpp#L473]
> at the same time.
--
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
[jira] [Commented] (PROTON-2802) [protonj2] Add additional validation scripting to the AMQP test peer
[ https://issues.apache.org/jira/browse/PROTON-2802?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17824208#comment-17824208 ] ASF subversion and git services commented on PROTON-2802: - Commit cf3667444bce4ff9da8c9097d7c7a2fa2f767386 in qpid-protonj2's branch refs/heads/main from Timothy Bish [ https://gitbox.apache.org/repos/asf?p=qpid-protonj2.git;h=cf366744 ] PROTON-2802 Make scripting message payload of a transfer simpler Provide a simpler API for scripting the expected message payload that will accompany an incoming transfer and provide some updates to match that API on the remote inject of transfer with message payloads. > [protonj2] Add additional validation scripting to the AMQP test peer > > > Key: PROTON-2802 > URL: https://issues.apache.org/jira/browse/PROTON-2802 > Project: Qpid Proton > Issue Type: Improvement > Components: protonj2 >Affects Versions: protonj2-1.0.0-M19 >Reporter: Timothy A. Bish >Assignee: Timothy A. Bish >Priority: Minor > Fix For: protonj2-1.0.0-M20 > > > Expand the test scripting APIs to make test writing easier in some cases -- 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
[jira] [Commented] (PROTON-2794) [protonj2] Transfer ID does not wrap as expected
[
https://issues.apache.org/jira/browse/PROTON-2794?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17824289#comment-17824289
]
Arnaud Cogoluègnes commented on PROTON-2794:
Thanks for the follow-up. I confirm it is working now if a peer chooses a large
next-outgoing-id when starting a new session.
> [protonj2] Transfer ID does not wrap as expected
>
>
> Key: PROTON-2794
> URL: https://issues.apache.org/jira/browse/PROTON-2794
> Project: Qpid Proton
> Issue Type: Bug
> Components: protonj2
>Affects Versions: protonj2-1.0.0-M19
>Reporter: Arnaud Cogoluègnes
>Assignee: Timothy A. Bish
>Priority: Major
> Fix For: protonj2-1.0.0-M20
>
>
> When ProtonJ 2 creates a session and the peer replies with a large
> {{next-outgoing-id}} in the {{begin}} response (e.g. 4294967292), a receiver
> on the session throws an exception later on when it detects the
> {{next-incoming-id}} is greater than the limit of a uint32.
> Corresponding code:
> [https://github.com/apache/qpid-protonj2/blob/d5144af7818b2695747782bacc410580d3ec7943/protonj2/src/main/java/org/apache/qpid/protonj2/types/transport/Flow.java#L153-L155]
> [https://github.com/apache/qpid-protonj2/blob/d5144af7818b2695747782bacc410580d3ec7943/protonj2/src/main/java/org/apache/qpid/protonj2/engine/impl/ProtonSessionIncomingWindow.java#L138]
> The {{transfer-id}} is a {{sequence-no}}, so it must follow
> [RFC-1982|https://www.ietf.org/rfc/rfc1982.txt] and so wrap around (sections
> 2.8.9 and 2.8.10 of the AMQP 1.0 spec).
> So from my understanding, the {{nextIncomingId}} field should be set to 0
> when it "overflows" and no exception should be thrown.
> Other fields like {{delivery-count}} may have the same problem.
--
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
