You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2021/04/11 12:32:23 UTC
[GitHub] [qpid-proton] DreamPearl opened a new pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
DreamPearl opened a new pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309
Work In Progress
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r612730769
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
I added a test `cpp/src/delivery_test.cpp` for delivery tag accessor but it looks too simple. Should I add a mutator for full testing? WDYT? @jiridanek
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r613520105
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,17 @@ namespace {
uint64_t tag_counter = 0;
}
-tracker sender::send(const message &message) {
+tracker sender::send(const message &message,const binary &tag) {
+ pn_delivery_t *dlv;
+ if(!tag.empty())
Review comment:
I didn't use the clang-format earlier, just did it. Thanks :)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] gemmellr commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616479712
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
@jiridanek Is the C++ binding sender actually considered thread-safe? I definitely didnt believe it to be, I think none of them are, and the C stuff its calling underneath certainly isnt. If its not then the tag generation doesnt seem a concern. The [pre-existing] approach used clearly isnt thread safe, multiple threads cant safely pre-/post-increment a simple shared value without some protection, but thats fine if it isnt intended or needed to be. The link usage in the send method also appears unprotected to my [admittedly C++ oblivious] eyes, which would mean its all inherently unsafe anyway, and duplicate tags is likely on the lesser end of issues that could come up in there.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616151542
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
> No, I haven't used it. Will look into it.
OK, let me give you a few pointers. If you already use an IDE, you should use something built-in into it. If you use e.g. VS Code, there should be a debugger for C++ as a plugin. It is good to use a debugger in a program you already know how to use. Learning a new IDE at the same time makes things hard. What IDE/editor are you using for C++?
Here's a tutorial for VS Code. Looking at it, I feel QT Creator was simpler ;P https://www.youtube.com/watch?v=G9gnSGKYIg4
When I had C++ at school, we used the QT Creator IDE which was quite nice, or Clion is ok, if you have a free licence from your school (which we had). Another possibility is Eclipse for C/C++ which @ganeshmurthy is probably still using.
There is a command-line debugger GDB (and LLDB, which looks essentially the same). Pretty much all the other graphical debuggers are simply graphical front ends for GDB. I do not recommend using GDB directly when you have another option.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r612761509
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
I think so. If you look at the tests that @pjfawcett wrote for these two enhancements https://github.com/apache/qpid-proton/pulls?q=is%3Apr+author%3Apjfawcett , you'll see that there are tests that set something when a message is being sent/link is being created/... and then the tests check it on the receiving end. **I think that this would be a good test in addition to what you have already. It demonstrates that an end-user of the library is actually able to use the tags.**
As you know, in AMQP you can set a tag when you are sending a message and you will see the tag in the receiver when your program decides whether to accept the message, and then again the tag is given back to the sender in the delivery after the message has been disposed of.
Looking at the C++ binding, I see that when I send a message, I won't be able to set my own tag, because the binding sets it for me (and tries moderately hard to give unique tag). For example, in the Python binding, there is `def send(self, sender, tag=None):` method in `message.py` so there I can set my own tag.
https://github.com/apache/qpid-proton/blob/8ddf5399013b02f1fd13bda3fee7886bd48a98be/cpp/src/sender.cpp#L62-L72
When I receive the message, I will be able to read the delivery tag in the `on_message` handler method
https://github.com/apache/qpid-proton/blob/8ddf5399013b02f1fd13bda3fee7886bd48a98be/cpp/examples/flow_control.cpp#L181
Finally, when the message is settled in the `on_message` handler of the receiver (using one of the `.accept()`, `reject()`, ... methods, I want to be able to see in the sender the final disposition and the tag. That should cause the handler method `on_tracker_accept`/`on_tracker_reject`/... to be called. **Problem is, I cannot access the delivery and therefore the delivery from that method, at least I did not see a way!**
https://github.com/apache/qpid-proton/blob/8ddf5399013b02f1fd13bda3fee7886bd48a98be/cpp/examples/direct_send.cpp#L76
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616484981
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
Thread safe is a matter of degree, of course. The situation I am thinking about is program with two threads, each running its own container and not interacting in any way with the other thread. I believed this is something that Proton supports or at least strives to support. In this case, because `uint64_t tag_counter = 0;` is a global variable, the two threads will step over each other's toes anyways and will send duplicate tags.
My suggestion for Raki here was to create a Jira ticket for it. Do you think what's happening is completely expected and raising Jira is not warranted? I fully agree that the issue has low priority. It has been this way for ages and noone ever complained.
edit: my c++ is not very good either, I am reviewing this with the belief that some inexpert early feedback is better than expert feedback some time later... hopefully I am not making a too big a fool of myself and you all
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615583489
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,115 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <iostream>
+#include <map>
+
+#include <cstdio>
+#include <cstdlib>
+#include <ctime>
+#include <sstream>
+#include <string>
+
+class direct_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ direct_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+
+ std::cout << msg.body() << std::endl;
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ }
+};
+
+class simple_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ simple_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ proton::connection_options co;
+ sender = c.open_sender(url, co);
+ }
+
+ void on_sendable(proton::sender &s) PN_CPP_OVERRIDE {
+ proton::message msg;
+ std::string m = "testing";
+ msg.body(m);
+ s.send(msg);
+ }
+};
+
+int test_delivery() {
+ std::string address("127.0.0.1:5672/examples");
+ direct_recv recv(address);
+ proton::container(recv).run();
Review comment:
Looks basically ok. Just fix the ThreadSanitizer warning I pasted.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-824691409
@DreamPearl Now close the Jira, PROTON-2370. It should have yourself as assignee, the next upcoming release as the Fix Version and status should be **Resolved*** (not Closed, that will be set upon release).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616446834
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
> If you already use an IDE, you should use something built-in into it. If you use e.g. VS Code, there should be a debugger for C++ as a plugin. It is good to use a debugger in a program you already know how to use. Learning a new IDE at the same time makes things hard. What IDE/editor are you using for C++?
I am using VS Code for C++.
> Here's a tutorial for VS Code. Looking at it, I feel QT Creator was simpler ;P https://www.youtube.com/watch?v=G9gnSGKYIg4
Thank you :) I will look into it.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615585457
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
Looks like the PR is basically there. Just fix the ThreadSanitizer warning and decide if you want to tackle the `tracker` here or in a new PR after this one (I'd say now add the handler methods and assert what you can; leave updating the tracker API (to make tag accessible there) for a new task).
Then we can ask one of the Proton C++ devs to review it.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r611226330
##########
File path: cpp/include/proton/tag.hpp
##########
@@ -0,0 +1,57 @@
+#ifndef PROTON_TAG_HPP
+#define PROTON_TAG_HPP
+
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include "./fwd.hpp"
+#include "./internal/export.hpp"
+#include "./endpoint.hpp"
+#include "./receiver.hpp"
+#include "./sender.hpp"
+
+#include <string>
+
+/// @file
+/// @copybrief proton::tag
+
+// struct pn_delivery_tag_t;
+
+namespace proton {
+
+/// A container of tag.
+class
+PN_CPP_CLASS_EXTERN tag : public internal::object<pn_delivery_tag_t> {
+ public:
+ /// @cond INTERNAL
+ PN_CPP_EXTERN tag(pn_delivery_tag_t* t) : internal::object<pn_delivery_tag_t>(t) {}
Review comment:
You should be able to include the header that defines the type, I added the following on top of the file and the compile error went away
```
#include "proton/delivery.h"
```
You can include C headers in a C++ code like this, because the delivery.h header has the following code on top, https://docs.microsoft.com/en-us/cpp/cpp/extern-cpp?view=msvc-160#extern-c-and-extern-c-function-declarations
```
#ifdef __cplusplus
extern "C" {
#endif
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616151542
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
> No, I haven't used it. Will look into it.
OK, let me give you a few pointers. If you already use an IDE, you should use something built-in into it. If you use e.g. VS Code, there should be a debugger for C++ as a plugin. It is good to use a debugger in a program you already know how to use. Learning a new IDE at the same time makes things hard. What IDE/editor are you using for C++?
Here's a tutorial for VS Code. Looking at it, I feel QT Creator was simpler ;P https://www.youtube.com/watch?v=G9gnSGKYIg4
When I had C++ at school, we used the free QT Creator IDE which was quite nice, or Clion is ok, if you have a free licence from your school (which we had). Another possibility is Eclipse for C/C++ which @ganeshmurthy is probably still using.
There is a command-line debugger GDB (and LLDB, which looks essentially the same). Pretty much all the other graphical debuggers are simply graphical front ends for GDB. I do not recommend using GDB directly when you have another option.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r612761509
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
I think so. If you look at the tests that @pjfawcett wrote for these two enhancements https://github.com/apache/qpid-proton/pulls?q=is%3Apr+author%3Apjfawcett , you'll see that there are tests that set something when a message is being sent/link is being created/... and then the tests check it on the receiving end. **I think that this would be a good test in addition to what you have already. It demonstrates that an end-user of the library is actually able to use the tags.**
As you know, in AMQP you can set a tag when you are sending a message and you will see the tag in the receiver when your program decides whether to accept the message, and then again the tag is given back to the sender in the delivery after the message has been disposed of.
Looking at the C++ binding, I see that when I send a message, I won't be able to set my own tag, because the binding sets it for me (and tries moderately hard to give unique tag). For example, in the Python binding, there is `def send(self, sender, tag=None):` method in `message.py` so there I can set my own tag.
https://github.com/apache/qpid-proton/blob/8ddf5399013b02f1fd13bda3fee7886bd48a98be/cpp/src/sender.cpp#L62-L72
When I receive the message, I will be able to read the delivery tag in the `on_message` handler method
https://github.com/apache/qpid-proton/blob/8ddf5399013b02f1fd13bda3fee7886bd48a98be/cpp/examples/flow_control.cpp#L181
Finally, when the message is settled in the `on_message` handler of the receiver (using one of the `.accept()`, `reject()`, ... methods, I want to be able to see in the sender the final disposition and the tag. That should cause the handler method `on_tracker_accept`/`on_tracker_reject`/... to be called. **Problem is, I cannot access the delivery and therefore the delivery tag from that method, at least I did not see a way!**
https://github.com/apache/qpid-proton/blob/8ddf5399013b02f1fd13bda3fee7886bd48a98be/cpp/examples/direct_send.cpp#L76
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r611878305
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
I started seeing error after make clean :( will take a look again.
```In file included from /home/rakhi/qpid-proton/cpp/src/delivery.cpp:28:
/home/rakhi/qpid-proton/cpp/src/proton_bits.hpp: In instantiation of ‘U proton::make_wrapper(typename proton::internal::wrapped<T>::type) [with U = proton::tag; typename proton::internal::wrapped<T>::type = pn_bytes_t]’:
/home/rakhi/qpid-proton/cpp/src/delivery.cpp:45:88: required from here
/home/rakhi/qpid-proton/cpp/src/proton_bits.hpp:159:91: error: cannot convert ‘proton::internal::wrapped<proton::tag>::type’ {aka ‘pn_bytes_t’} to ‘proton::internal::wrapped<proton::tag>::type*’ {aka ‘pn_bytes_t*’}
159 | U make_wrapper(typename internal::wrapped<U>::type t) { return internal::factory<U>::wrap(t); }
| ^
| |
| proton::internal::wrapped<proton::tag>::type {aka pn_bytes_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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] gemmellr commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r613149341
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
I would think it would be fine for a binding to generate one by default, with then perhaps a separate ability to set it yourself if you wanted (many probably wont need to).
The set of tags needs to be unique for deliveries considered unsettled by either end of a given link. You can reuse tags from previously settled deliveries if you like. Different links can use the same tags at the same time.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] codecov-commenter edited a comment on pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-822450129
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615813070
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,132 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ listener_port = l.port();
+ listener_ready = true;
Review comment:
Thanks, I was missing mutex locking just after opening listener and unlocking before notifying :'(
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r612580842
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
@ssorj I think I now see what you meant, the `pn_delivery_tag` is a type alias for `pn_bytes_t`, and the reasonable types for in in C++ are std::string or std::vector<uint8>. So, one of the two following functions should be used instead of `make_wrapper`
https://github.com/apache/qpid-proton/blob/8ddf5399013b02f1fd13bda3fee7886bd48a98be/cpp/src/types_internal.hpp#L58-L59
Both make a copy of the data, which simplifies things.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] codecov-commenter edited a comment on pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-822450129
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r612765958
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
@ssorj For your example, is is necessary to set custom delivery tags when sending messages, or is it ok to have the tags generated by the binding, and then make them available read-only? Do you need to be absolutely sure that delivery tags are unique?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615825219
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::unique_lock<std::mutex> lk(m);
+ std::cout << "listening on " << l.port() << std::endl;
Review comment:
Done. Thanks!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615578902
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,132 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ listener_port = l.port();
+ listener_ready = true;
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ proton::binary test_tag_recv("TESTTAG");
+ ASSERT_EQUAL(test_tag_recv, d.tag());
+ }
+};
+
+class test_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ test_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ proton::connection_options co;
+ sender = c.open_sender(url, co);
+ }
+
+ void on_sendable(proton::sender &s) PN_CPP_OVERRIDE {
+ proton::message msg;
+ msg.body("message");
+ proton::binary test_tag_send("TESTTAG");
+ s.send(msg, test_tag_send);
+ }
Review comment:
You do not have a handler for when the delivery is settled, where you'd also check the tag value. As far as I can tell, you cannot access the tag from a `tracker`. If you think that making tag accessible there should be done in a new Jira, that is ok with me too.
```
// called in the receiver
void on_delivery_settle(proton::delivery &delivery) override {
}
// called in the sender
void on_tracker_accept(proton::tracker &tracker) override {
}
void on_tracker_settle(proton::tracker &tracker) override {
}
```
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,132 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
Review comment:
Good thing Proton Cpp is moving towards C++11 for the next release. Otherwise you wouldn't be able to use these. Now it's fine!
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,132 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ listener_port = l.port();
+ listener_ready = true;
Review comment:
https://travis-ci.com/github/apache/qpid-proton/jobs/499340056#L5205
```
23: Test command: /opt/pyenv/shims/python3 "/home/travis/build/apache/qpid-proton/scripts/env.py" "--" "TSAN_OPTIONS=second_deadlock_stack=1 suppressions=/home/travis/build/apache/qpid-proton/tests/tsan.supp" "PN_SASL_CONFIG_PATH=/home/travis/build/apache/qpid-proton/build/cpp/testdata/sasl-conf" "/home/travis/build/apache/qpid-proton/build/cpp/delivery_test"
23: Test timeout computed to be: 360
23: TEST: test_delivery_tag()
23: listening on 43505
23: ==================
23: WARNING: ThreadSanitizer: data race (pid=14832)
23: Write of size 1 at 0x555ab72fc164 by thread T1:
23: #0 test_recv::listener_ready_handler::on_open(proton::listener&) /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:62 (delivery_test+0x7fee)
23: #1 proton::container::impl::dispatch(pn_event_t*) /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:602 (libqpid-proton-cpp.so.12+0x4d02d)
23: #2 proton::container::impl::thread() /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:765 (libqpid-proton-cpp.so.12+0x4d567)
23: #3 proton::container::impl::run(int) /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:812 (libqpid-proton-cpp.so.12+0x4db6b)
23: #4 proton::container::run() /home/travis/build/apache/qpid-proton/cpp/src/container.cpp:92 (libqpid-proton-cpp.so.12+0x3587b)
23: #5 operator() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:113 (delivery_test+0x6628)
23: #6 __invoke_impl<void, test_delivery_tag()::<lambda()> > /usr/include/c++/10/bits/invoke.h:60 (delivery_test+0x6628)
23: #7 __invoke<test_delivery_tag()::<lambda()> > /usr/include/c++/10/bits/invoke.h:95 (delivery_test+0x6628)
23: #8 _M_invoke<0> /usr/include/c++/10/thread:264 (delivery_test+0x6628)
23: #9 operator() /usr/include/c++/10/thread:271 (delivery_test+0x6628)
23: #10 _M_run /usr/include/c++/10/thread:215 (delivery_test+0x6628)
23: #11 <null> <null> (libstdc++.so.6+0xd6d83)
23:
23: Previous read of size 1 at 0x555ab72fc164 by main thread (mutexes: write M50):
23: #0 operator() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:117 (delivery_test+0x6c14)
23: #1 wait<test_delivery_tag()::<lambda()> > /usr/include/c++/10/condition_variable:110 (delivery_test+0x6c14)
23: #2 test_delivery_tag() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:117 (delivery_test+0x6c14)
23: #3 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: Location is global '(anonymous namespace)::listener_ready' of size 1 at 0x555ab72fc164 (delivery_test+0x00000000c164)
23:
23: Mutex M50 (0x555ab72fc1c0) created at:
23: #0 pthread_mutex_lock <null> (libtsan.so.0+0x5271c)
23: #1 __gthread_mutex_lock /usr/include/x86_64-linux-gnu/c++/10/bits/gthr-default.h:749 (delivery_test+0x6be6)
23: #2 std::mutex::lock() /usr/include/c++/10/bits/std_mutex.h:100 (delivery_test+0x6be6)
23: #3 std::unique_lock<std::mutex>::lock() /usr/include/c++/10/bits/unique_lock.h:138 (delivery_test+0x6be6)
23: #4 std::unique_lock<std::mutex>::unique_lock(std::mutex&) /usr/include/c++/10/bits/unique_lock.h:68 (delivery_test+0x6be6)
23: #5 test_delivery_tag() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:116 (delivery_test+0x6be6)
23: #6 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: Thread T1 (tid=14834, running) created by main thread at:
23: #0 pthread_create <null> (libtsan.so.0+0x5ea99)
23: #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) <null> (libstdc++.so.6+0xd7048)
23: #2 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: SUMMARY: ThreadSanitizer: data race /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:62 in test_recv::listener_ready_handler::on_open(proton::listener&)
23: ==================
23: ==================
23: WARNING: ThreadSanitizer: data race (pid=14832)
23: Read of size 4 at 0x555ab72fc160 by main thread (mutexes: write M50):
23: #0 test_delivery_tag() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:120 (delivery_test+0x6c4b)
23: #1 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: Previous write of size 4 at 0x555ab72fc160 by thread T1:
23: #0 test_recv::listener_ready_handler::on_open(proton::listener&) /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:61 (delivery_test+0x7fdb)
23: #1 proton::container::impl::dispatch(pn_event_t*) /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:602 (libqpid-proton-cpp.so.12+0x4d02d)
23: #2 proton::container::impl::thread() /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:765 (libqpid-proton-cpp.so.12+0x4d567)
23: #3 proton::container::impl::run(int) /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:812 (libqpid-proton-cpp.so.12+0x4db6b)
23: #4 proton::container::run() /home/travis/build/apache/qpid-proton/cpp/src/container.cpp:92 (libqpid-proton-cpp.so.12+0x3587b)
23: #5 operator() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:113 (delivery_test+0x6628)
23: #6 __invoke_impl<void, test_delivery_tag()::<lambda()> > /usr/include/c++/10/bits/invoke.h:60 (delivery_test+0x6628)
23: #7 __invoke<test_delivery_tag()::<lambda()> > /usr/include/c++/10/bits/invoke.h:95 (delivery_test+0x6628)
23: #8 _M_invoke<0> /usr/include/c++/10/thread:264 (delivery_test+0x6628)
23: #9 operator() /usr/include/c++/10/thread:271 (delivery_test+0x6628)
23: #10 _M_run /usr/include/c++/10/thread:215 (delivery_test+0x6628)
23: #11 <null> <null> (libstdc++.so.6+0xd6d83)
23:
23: Location is global '(anonymous namespace)::listener_port' of size 4 at 0x555ab72fc160 (delivery_test+0x00000000c160)
23:
23: Mutex M50 (0x555ab72fc1c0) created at:
23: #0 pthread_mutex_lock <null> (libtsan.so.0+0x5271c)
23: #1 __gthread_mutex_lock /usr/include/x86_64-linux-gnu/c++/10/bits/gthr-default.h:749 (delivery_test+0x6be6)
23: #2 std::mutex::lock() /usr/include/c++/10/bits/std_mutex.h:100 (delivery_test+0x6be6)
23: #3 std::unique_lock<std::mutex>::lock() /usr/include/c++/10/bits/unique_lock.h:138 (delivery_test+0x6be6)
23: #4 std::unique_lock<std::mutex>::unique_lock(std::mutex&) /usr/include/c++/10/bits/unique_lock.h:68 (delivery_test+0x6be6)
23: #5 test_delivery_tag() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:116 (delivery_test+0x6be6)
23: #6 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: Thread T1 (tid=14834, running) created by main thread at:
23: #0 pthread_create <null> (libtsan.so.0+0x5ea99)
23: #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) <null> (libstdc++.so.6+0xd7048)
23: #2 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: SUMMARY: ThreadSanitizer: data race /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:120 in test_delivery_tag()
23: ==================
23: ThreadSanitizer: reported 2 warnings
23/46 Test #23: cpp-delivery_test ................***Failed 0.15 sec
```
CI caught something about this like. Do you see what's wrong/different from the cppreference.com usage example :P ?
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
I am not aware and I could not find anything in Jira. If you can't find it either, then please file a new one. We need to ensure that a single sender will not send a message with a duplicate tag.
btw, I found this docs jira about auto settlement, https://issues.apache.org/jira/browse/PROTON-2060; it might become relevant here later.
I tried to make sure that the `tag_counter` idea really is as broken as I thought it is, so here is my test program
```
$ cat increment.cpp
#include <thread>
#include <cstdio>
#include <cstdlib>
int count = 0;
void func1()
{
for ( int i =0 ; i < 1000000; ++i )
count = count + 1;
}
void func2()
{
int prev = 0;
for ( int i =0 ; i < 1000000; ++i ) {
int neww = ++count;
if (prev == neww) {
printf("failure func2: prev %d, neww %d\n", prev, neww);
abort();
}
prev = neww;
}
}
void func3()
{
int prev = 0;
for ( int i =0 ; i < 1000000; ++i ) {
int neww = ++count;
if (prev == neww) {
printf("failure func3: prev %d neww %d\n", prev, neww);
abort();
}
prev = neww;
}
}
int main()
{
auto t1 = std::thread(func1);
auto t2 = std::thread(func2);
auto t3 = std::thread(func3);
//joining all the threads
t1.join();
t2.join();
t3.join();
printf("total: %d \n", count);
return 0;
}
```
and it looks that yes, it is possible to get the same tag twice in the same thread. So yes, it apparently can give you duplicate tags.
```
% g++ increment.cpp -lpthread && ./a.out
failure func3: prev 349602 neww 349602
[1] 9584 abort (core dumped) ./a.out
% g++ increment.cpp -lpthread && ./a.out
failure func2: prev 188384, neww 188384
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r619688242
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
It's working now. Thanks, Jiri. It really helped.
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
Thank you, ack :)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r613515581
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,17 @@ namespace {
uint64_t tag_counter = 0;
}
-tracker sender::send(const message &message) {
+tracker sender::send(const message &message,const binary &tag) {
+ pn_delivery_t *dlv;
+ if(!tag.empty())
+ {
+ dlv = pn_delivery(pn_object(), pn_dtag((std::string(tag)).c_str(), tag.size()));
Review comment:
(but use a c++-style cast, I did not do it above because I don't know which one is appropriate)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] gemmellr commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616492587
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
> In this case, because `uint64_t tag_counter = 0;` is a global variable, the two threads will step over each other's toes anyways and will send duplicate tags.
Ah ok. Yes they would in that case. I wasnt realising it was global, I thought it was a sender-specific variable in the senders case (I told you my eyes are C++ oblivious). If its global thats clearly a bug that should be raised, then it can be fixed in isolation.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r619691844
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
Thank you, ack :)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-824701452
> @DreamPearl Now close the Jira, PROTON-2370. It should have yourself as assignee, the next upcoming release as the Fix Version and status should be **Resolved** (not Closed, that will be set upon release).
Done. Thanks!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] codecov-commenter edited a comment on pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-822450129
# [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#309](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f2ed11e) into [main](https://codecov.io/gh/apache/qpid-proton/commit/19ae83a1ef3ecf34c34aae72f2a6a878f138749c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (19ae83a) will **increase** coverage by `19.30%`.
> The diff coverage is `n/a`.
> :exclamation: Current head f2ed11e differs from pull request most recent head c67ba09. Consider uploading reports for the commit c67ba09 to get more accurate results
[](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## main #309 +/- ##
===========================================
+ Coverage 69.03% 88.34% +19.30%
===========================================
Files 351 47 -304
Lines 69481 2394 -67087
===========================================
- Hits 47967 2115 -45852
+ Misses 21514 279 -21235
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [cpp/examples/multithreaded\_client\_flow\_control.cpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL2V4YW1wbGVzL211bHRpdGhyZWFkZWRfY2xpZW50X2Zsb3dfY29udHJvbC5jcHA=) | | |
| [cpp/examples/simple\_connect.cpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL2V4YW1wbGVzL3NpbXBsZV9jb25uZWN0LmNwcA==) | | |
| [cpp/include/proton/map.hpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL2luY2x1ZGUvcHJvdG9uL21hcC5ocHA=) | | |
| [c/examples/broker.c](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Yy9leGFtcGxlcy9icm9rZXIuYw==) | | |
| [python/proton/\_handlers.py](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHl0aG9uL3Byb3Rvbi9faGFuZGxlcnMucHk=) | | |
| [cpp/src/connect\_config\_test.cpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL3NyYy9jb25uZWN0X2NvbmZpZ190ZXN0LmNwcA==) | | |
| [cpp/include/proton/type\_id.hpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL2luY2x1ZGUvcHJvdG9uL3R5cGVfaWQuaHBw) | | |
| [cpp/src/sasl.cpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL3NyYy9zYXNsLmNwcA==) | | |
| [cpp/src/container\_test.cpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL3NyYy9jb250YWluZXJfdGVzdC5jcHA=) | | |
| [python/tests/proton\_tests/ssl.py](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHl0aG9uL3Rlc3RzL3Byb3Rvbl90ZXN0cy9zc2wucHk=) | | |
| ... and [288 more](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [19ae83a...c67ba09](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-821532591
> I am still working on fixing tests.
Sure. My point was that when you have a CI available, like here, it's best to first get the PR compiling. That way you get some feedback that the PR is ok on some level (that at least it compiles) and avoid some unpleasant surprises later, as you continue working.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] codecov-commenter edited a comment on pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-822450129
# [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#309](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f2ed11e) into [main](https://codecov.io/gh/apache/qpid-proton/commit/19ae83a1ef3ecf34c34aae72f2a6a878f138749c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (19ae83a) will **increase** coverage by `16.38%`.
> The diff coverage is `n/a`.
> :exclamation: Current head f2ed11e differs from pull request most recent head c67ba09. Consider uploading reports for the commit c67ba09 to get more accurate results
[](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## main #309 +/- ##
===========================================
+ Coverage 69.03% 85.42% +16.38%
===========================================
Files 351 47 -304
Lines 69481 2394 -67087
===========================================
- Hits 47967 2045 -45922
+ Misses 21514 349 -21165
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [ruby/lib/core/message.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS9tZXNzYWdlLnJi) | `57.05% <0.00%> (-31.77%)` | :arrow_down: |
| [ruby/lib/util/error\_handler.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvdXRpbC9lcnJvcl9oYW5kbGVyLnJi) | `59.37% <0.00%> (-28.13%)` | :arrow_down: |
| [ruby/spec/spec\_helper.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9zcGVjL3NwZWNfaGVscGVyLnJi) | `54.71% <0.00%> (-9.44%)` | :arrow_down: |
| [ruby/lib/codec/data.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29kZWMvZGF0YS5yYg==) | `95.21% <0.00%> (-0.87%)` | :arrow_down: |
| [cpp/examples/server\_direct.cpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL2V4YW1wbGVzL3NlcnZlcl9kaXJlY3QuY3Bw) | | |
| [cpp/src/proactor\_container\_impl.hpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL3NyYy9wcm9hY3Rvcl9jb250YWluZXJfaW1wbC5ocHA=) | | |
| [cpp/examples/broker.cpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL2V4YW1wbGVzL2Jyb2tlci5jcHA=) | | |
| [cpp/include/proton/listen\_handler.hpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL2luY2x1ZGUvcHJvdG9uL2xpc3Rlbl9oYW5kbGVyLmhwcA==) | | |
| [cpp/src/scalar\_test.cpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL3NyYy9zY2FsYXJfdGVzdC5jcHA=) | | |
| [c/src/core/transport.c](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Yy9zcmMvY29yZS90cmFuc3BvcnQuYw==) | | |
| ... and [292 more](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [19ae83a...c67ba09](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615100630
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,115 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <iostream>
+#include <map>
+
+#include <cstdio>
+#include <cstdlib>
+#include <ctime>
+#include <sstream>
+#include <string>
+
+class direct_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ direct_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+
+ std::cout << msg.body() << std::endl;
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ }
+};
+
+class simple_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ simple_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ proton::connection_options co;
+ sender = c.open_sender(url, co);
+ }
+
+ void on_sendable(proton::sender &s) PN_CPP_OVERRIDE {
+ proton::message msg;
+ std::string m = "testing";
+ msg.body(m);
+ s.send(msg);
+ }
+};
+
+int test_delivery() {
+ std::string address("127.0.0.1:5672/examples");
Review comment:
hardcoded port; we'd like to allocate a random free port to avoid port clashes. but that is something for later, for now, just focus on getting the test to pass, you can improve it later
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r611226640
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
This does not compile for me too. The reason seems to be that `make_wrapper` expects to get a pointer type, but `pn_delivery_tag` returns the struct directly as a value, not a pointer to it. If I exactly knew right now what you should do here, I'd tell you...
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r611818315
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
@jiridanek I added an additional template in `cpp/src/proton_bits.hpp` and the error went away. Now I am able to build but I am not sure if we can do it like this, WDYT?
Meanwhile, I am trying writing tests so that we can know if it is working fine or not.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r614789081
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,17 @@ namespace {
uint64_t tag_counter = 0;
}
-tracker sender::send(const message &message) {
+tracker sender::send(const message &message,const binary &tag) {
+ pn_delivery_t *dlv;
+ if(!tag.empty())
+ {
+ dlv = pn_delivery(pn_object(), pn_dtag((std::string(tag)).c_str(), tag.size()));
Review comment:
Done. Thanks!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r619689825
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
Awesome. As a further improvement you could look into, there's probably something like these .env files, https://github.com/microsoft/vscode-python/issues/544. The idea is that you don't have to specify env variables in the launch config, which is quite clumsy, but instead you can keep them in a more readable format in a file. But I guess there was not too many environment variables necessary. Probably just that `LD_LIBRARY_PATH` I mentioned, right?
If you want some more work now, and to practice debugging ;P you could have a look at https://issues.apache.org/jira/browse/PROTON-2104. (Unless Justin finds you something better, or it's already time to look into your main task.) If you recall your tests for the url class, there were some tests for IPv6, and they were passing. So, how it's possible that IPv6 connections don't work? I tried stepping through the code in a debugger some time ago, and I left a comment. You'd have to double-check if the observations are still valid. Use the step-into command to follow the program execution, and see where the url string breaks. Then, you can propose a solution, where to best apply a fix.
## Motivational cartoon, just for fun ;)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r619688242
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
It's working now. Thanks, Jiri. It really helped.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615817623
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::unique_lock<std::mutex> lk(m);
+ std::cout << "listening on " << l.port() << std::endl;
Review comment:
few things:
* remove the "listening on" print, tests should be silent when they are running ok
* pull all code that does not have to be guarded by the lock outside of the locked section (that's the print, which you'll delete, so nothing to do here)
* consider using {} and RAII for the lock, so that you don't have to explicitly call unlock
```
{
std::lock_guard<std::mutex> lk(cv_m);
listener_port = l.port();
 listener_ready = true;
}
```
the lock will unlock itself after the {} block.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615455343
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
Should I file the bug for this or does it already exists?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615806807
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,132 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ listener_port = l.port();
+ listener_ready = true;
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ proton::binary test_tag_recv("TESTTAG");
+ ASSERT_EQUAL(test_tag_recv, d.tag());
+ }
+};
+
+class test_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ test_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ proton::connection_options co;
+ sender = c.open_sender(url, co);
+ }
+
+ void on_sendable(proton::sender &s) PN_CPP_OVERRIDE {
+ proton::message msg;
+ msg.body("message");
+ proton::binary test_tag_send("TESTTAG");
+ s.send(msg, test_tag_send);
+ }
Review comment:
Added the `on_delivery_settle()` method. Thanks.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl edited a comment on pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl edited a comment on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-821153641
> My guess: you created a new file tag.hpp and forgot to commit it?
I once created this file but later deleted it, then forgot to delete it from include :'( Fixed now.
> @DreamPearl It's probably a high time you fixed the CI failure https://github.com/apache/qpid-proton/pull/309/checks?check_run_id=2362084471#step:11:185
I am still working on fixing tests.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616151542
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
> No, I haven't used it. Will look into it.
OK, let me give you a few pointers. If you already use an IDE, you should use something built-in into it. If you use e.g. VS Code, there should be a debugger for C++ as a plugin. It is good to use a debugger in a program you already know how to use. Learning a new IDE at the same time makes things hard. What IDE/editor are you using for C++?
Here's a tutorial for VS Code. Looking at it, I feel QT Creator was simpler ;P https://www.youtube.com/watch?v=G9gnSGKYIg4
When I had C++ at school, we used the free QT Creator IDE which was quite nice, or Clion is ok, if you have a Clion licence from your school (which we had). Another possibility is Eclipse for C/C++ which @ganeshmurthy is probably still using. I also used KDevelop at some point, and before it Anjuta.
There is a command-line debugger GDB (and LLDB, which looks essentially the same). Pretty much all the other graphical debuggers are simply graphical front ends for GDB. I do not recommend using GDB directly when you have another option.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615583669
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,115 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <iostream>
+#include <map>
+
+#include <cstdio>
+#include <cstdlib>
+#include <ctime>
+#include <sstream>
+#include <string>
+
+class direct_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ direct_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+
+ std::cout << msg.body() << std::endl;
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ }
+};
+
+class simple_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ simple_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ proton::connection_options co;
+ sender = c.open_sender(url, co);
+ }
+
+ void on_sendable(proton::sender &s) PN_CPP_OVERRIDE {
+ proton::message msg;
+ std::string m = "testing";
+ msg.body(m);
+ s.send(msg);
+ }
+};
+
+int test_delivery() {
+ std::string address("127.0.0.1:5672/examples");
Review comment:
:+1: exactly!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616091512
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
It looks like the ASSERT_EQUAL that is right after those three lines is running but the ASSERT_EQUAL that is inside `on_delivery_settle` is not. I tried to fix that one but I am not able to understand when `on_delivery_settle` will be called, as on debugging `on_delivery_settle` is not even getting called.
I tried to investigate more about `on_delivery_settle`, and I found an archived thread in the user mailing list related to it, I am not sure if that's related to the issue I am facing.
https://www.mail-archive.com/users@qpid.apache.org/msg15078.html
Meanwhile investigating more.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616144888
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
> The execution goes through line 133 there, so it misses the only call to `on_delivery_settle` in the entire codebase below in the else if branch.
Oh okay, got it! Removed it. Thanks!
>Do you have a debugger too? My first thought would be to go for a debugger; it does not give such a persuasive screenshot that you've provided, but on the other hand it actually tells you what's happening line after line...
No, I haven't used it. Will look into it.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r617712959
##########
File path: cpp/src/sender.cpp
##########
@@ -66,8 +66,15 @@ uint64_t tag_counter = 0;
tracker sender::send(const message &message) {
uint64_t id = ++tag_counter;
- pn_delivery_t *dlv =
- pn_delivery(pn_object(), pn_dtag(reinterpret_cast<const char*>(&id), sizeof(id)));
+ const binary tag(
+ std::string(reinterpret_cast<const char *>(&id), sizeof(id)));
Review comment:
I don't much like bringing in `std::string`. What about this?
```
const uint8_t *begin = reinterpret_cast<const uint8_t *>(&id);
const binary tag(begin, begin + sizeof(id));
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r613508380
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,17 @@ namespace {
uint64_t tag_counter = 0;
}
-tracker sender::send(const message &message) {
+tracker sender::send(const message &message,const binary &tag) {
+ pn_delivery_t *dlv;
+ if(!tag.empty())
Review comment:
this is formatting that clang-format gave you? we'll need to configure the defaults a bit, because in Proton we put braces on the same line, not after `if`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615458277
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,115 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <iostream>
+#include <map>
+
+#include <cstdio>
+#include <cstdlib>
+#include <ctime>
+#include <sstream>
+#include <string>
+
+class direct_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ direct_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+
+ std::cout << msg.body() << std::endl;
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ }
+};
+
+class simple_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ simple_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ proton::connection_options co;
+ sender = c.open_sender(url, co);
+ }
+
+ void on_sendable(proton::sender &s) PN_CPP_OVERRIDE {
+ proton::message msg;
+ std::string m = "testing";
+ msg.body(m);
+ s.send(msg);
+ }
+};
+
+int test_delivery() {
+ std::string address("127.0.0.1:5672/examples");
Review comment:
Done. I used `port number 0` to automatically find the free port.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615101682
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,115 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <iostream>
+#include <map>
+
+#include <cstdio>
+#include <cstdlib>
+#include <ctime>
+#include <sstream>
+#include <string>
+
+class direct_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ direct_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+
+ std::cout << msg.body() << std::endl;
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ }
+};
+
+class simple_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ simple_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ proton::connection_options co;
+ sender = c.open_sender(url, co);
+ }
+
+ void on_sendable(proton::sender &s) PN_CPP_OVERRIDE {
+ proton::message msg;
+ std::string m = "testing";
+ msg.body(m);
+ s.send(msg);
+ }
+};
+
+int test_delivery() {
+ std::string address("127.0.0.1:5672/examples");
+ direct_recv recv(address);
+ proton::container(recv).run();
+ simple_send send(address);
+ proton::container(send).run();
+
+ return 0;
+}
+
+int main(int argc, char **argv) {
+ int failed = 0;
+ RUN_ARGV_TEST(failed, test_delivery());
Review comment:
you surely noticed that there are two ways of writing tests in Proton cpp. One way uses the catch 1.x framework, and the other type of tests use these custom macros. You worked with Catch in your first task, here you are using the custom framework. Either is fine to use, at this point. Don't rewrite the test to Catch just because of this comment.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615581743
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,132 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ listener_port = l.port();
+ listener_ready = true;
Review comment:
https://travis-ci.com/github/apache/qpid-proton/jobs/499340056#L5205
```
23: Test command: /opt/pyenv/shims/python3 "/home/travis/build/apache/qpid-proton/scripts/env.py" "--" "TSAN_OPTIONS=second_deadlock_stack=1 suppressions=/home/travis/build/apache/qpid-proton/tests/tsan.supp" "PN_SASL_CONFIG_PATH=/home/travis/build/apache/qpid-proton/build/cpp/testdata/sasl-conf" "/home/travis/build/apache/qpid-proton/build/cpp/delivery_test"
23: Test timeout computed to be: 360
23: TEST: test_delivery_tag()
23: listening on 43505
23: ==================
23: WARNING: ThreadSanitizer: data race (pid=14832)
23: Write of size 1 at 0x555ab72fc164 by thread T1:
23: #0 test_recv::listener_ready_handler::on_open(proton::listener&) /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:62 (delivery_test+0x7fee)
23: #1 proton::container::impl::dispatch(pn_event_t*) /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:602 (libqpid-proton-cpp.so.12+0x4d02d)
23: #2 proton::container::impl::thread() /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:765 (libqpid-proton-cpp.so.12+0x4d567)
23: #3 proton::container::impl::run(int) /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:812 (libqpid-proton-cpp.so.12+0x4db6b)
23: #4 proton::container::run() /home/travis/build/apache/qpid-proton/cpp/src/container.cpp:92 (libqpid-proton-cpp.so.12+0x3587b)
23: #5 operator() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:113 (delivery_test+0x6628)
23: #6 __invoke_impl<void, test_delivery_tag()::<lambda()> > /usr/include/c++/10/bits/invoke.h:60 (delivery_test+0x6628)
23: #7 __invoke<test_delivery_tag()::<lambda()> > /usr/include/c++/10/bits/invoke.h:95 (delivery_test+0x6628)
23: #8 _M_invoke<0> /usr/include/c++/10/thread:264 (delivery_test+0x6628)
23: #9 operator() /usr/include/c++/10/thread:271 (delivery_test+0x6628)
23: #10 _M_run /usr/include/c++/10/thread:215 (delivery_test+0x6628)
23: #11 <null> <null> (libstdc++.so.6+0xd6d83)
23:
23: Previous read of size 1 at 0x555ab72fc164 by main thread (mutexes: write M50):
23: #0 operator() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:117 (delivery_test+0x6c14)
23: #1 wait<test_delivery_tag()::<lambda()> > /usr/include/c++/10/condition_variable:110 (delivery_test+0x6c14)
23: #2 test_delivery_tag() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:117 (delivery_test+0x6c14)
23: #3 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: Location is global '(anonymous namespace)::listener_ready' of size 1 at 0x555ab72fc164 (delivery_test+0x00000000c164)
23:
23: Mutex M50 (0x555ab72fc1c0) created at:
23: #0 pthread_mutex_lock <null> (libtsan.so.0+0x5271c)
23: #1 __gthread_mutex_lock /usr/include/x86_64-linux-gnu/c++/10/bits/gthr-default.h:749 (delivery_test+0x6be6)
23: #2 std::mutex::lock() /usr/include/c++/10/bits/std_mutex.h:100 (delivery_test+0x6be6)
23: #3 std::unique_lock<std::mutex>::lock() /usr/include/c++/10/bits/unique_lock.h:138 (delivery_test+0x6be6)
23: #4 std::unique_lock<std::mutex>::unique_lock(std::mutex&) /usr/include/c++/10/bits/unique_lock.h:68 (delivery_test+0x6be6)
23: #5 test_delivery_tag() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:116 (delivery_test+0x6be6)
23: #6 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: Thread T1 (tid=14834, running) created by main thread at:
23: #0 pthread_create <null> (libtsan.so.0+0x5ea99)
23: #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) <null> (libstdc++.so.6+0xd7048)
23: #2 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: SUMMARY: ThreadSanitizer: data race /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:62 in test_recv::listener_ready_handler::on_open(proton::listener&)
23: ==================
23: ==================
23: WARNING: ThreadSanitizer: data race (pid=14832)
23: Read of size 4 at 0x555ab72fc160 by main thread (mutexes: write M50):
23: #0 test_delivery_tag() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:120 (delivery_test+0x6c4b)
23: #1 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: Previous write of size 4 at 0x555ab72fc160 by thread T1:
23: #0 test_recv::listener_ready_handler::on_open(proton::listener&) /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:61 (delivery_test+0x7fdb)
23: #1 proton::container::impl::dispatch(pn_event_t*) /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:602 (libqpid-proton-cpp.so.12+0x4d02d)
23: #2 proton::container::impl::thread() /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:765 (libqpid-proton-cpp.so.12+0x4d567)
23: #3 proton::container::impl::run(int) /home/travis/build/apache/qpid-proton/cpp/src/proactor_container_impl.cpp:812 (libqpid-proton-cpp.so.12+0x4db6b)
23: #4 proton::container::run() /home/travis/build/apache/qpid-proton/cpp/src/container.cpp:92 (libqpid-proton-cpp.so.12+0x3587b)
23: #5 operator() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:113 (delivery_test+0x6628)
23: #6 __invoke_impl<void, test_delivery_tag()::<lambda()> > /usr/include/c++/10/bits/invoke.h:60 (delivery_test+0x6628)
23: #7 __invoke<test_delivery_tag()::<lambda()> > /usr/include/c++/10/bits/invoke.h:95 (delivery_test+0x6628)
23: #8 _M_invoke<0> /usr/include/c++/10/thread:264 (delivery_test+0x6628)
23: #9 operator() /usr/include/c++/10/thread:271 (delivery_test+0x6628)
23: #10 _M_run /usr/include/c++/10/thread:215 (delivery_test+0x6628)
23: #11 <null> <null> (libstdc++.so.6+0xd6d83)
23:
23: Location is global '(anonymous namespace)::listener_port' of size 4 at 0x555ab72fc160 (delivery_test+0x00000000c160)
23:
23: Mutex M50 (0x555ab72fc1c0) created at:
23: #0 pthread_mutex_lock <null> (libtsan.so.0+0x5271c)
23: #1 __gthread_mutex_lock /usr/include/x86_64-linux-gnu/c++/10/bits/gthr-default.h:749 (delivery_test+0x6be6)
23: #2 std::mutex::lock() /usr/include/c++/10/bits/std_mutex.h:100 (delivery_test+0x6be6)
23: #3 std::unique_lock<std::mutex>::lock() /usr/include/c++/10/bits/unique_lock.h:138 (delivery_test+0x6be6)
23: #4 std::unique_lock<std::mutex>::unique_lock(std::mutex&) /usr/include/c++/10/bits/unique_lock.h:68 (delivery_test+0x6be6)
23: #5 test_delivery_tag() /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:116 (delivery_test+0x6be6)
23: #6 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: Thread T1 (tid=14834, running) created by main thread at:
23: #0 pthread_create <null> (libtsan.so.0+0x5ea99)
23: #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) <null> (libstdc++.so.6+0xd7048)
23: #2 main /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:130 (delivery_test+0x646c)
23:
23: SUMMARY: ThreadSanitizer: data race /home/travis/build/apache/qpid-proton/cpp/src/delivery_test.cpp:120 in test_delivery_tag()
23: ==================
23: ThreadSanitizer: reported 2 warnings
23/46 Test #23: cpp-delivery_test ................***Failed 0.15 sec
```
CI caught something about this that it does not like. Do you see what's wrong/different from the cppreference.com usage example :P ?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615100296
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,115 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <iostream>
+#include <map>
+
+#include <cstdio>
+#include <cstdlib>
+#include <ctime>
+#include <sstream>
+#include <string>
+
+class direct_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ direct_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+
+ std::cout << msg.body() << std::endl;
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ }
+};
+
+class simple_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ simple_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ proton::connection_options co;
+ sender = c.open_sender(url, co);
+ }
+
+ void on_sendable(proton::sender &s) PN_CPP_OVERRIDE {
+ proton::message msg;
+ std::string m = "testing";
+ msg.body(m);
+ s.send(msg);
+ }
+};
+
+int test_delivery() {
+ std::string address("127.0.0.1:5672/examples");
+ direct_recv recv(address);
+ proton::container(recv).run();
Review comment:
This call to `run()` blocks. The receiver waits here for message and it never runs the lines below to actually send the message. I can think of two solutions: either start the receiver in a new thread, or use a single proton::container that opens both a sender and a receiver and sends a message to itself.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615099019
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
The `send()` method makes sense to me. If the API it good enough for Python, it should make some sense for C++ too ;P
I'll add few notes to the code. It is not something to be addressed right away. Those are just improvements I thought of. I agree that writing a test should be the thing to do next.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r612693197
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
Ohh so I was supposed to do it like this, Thanks Jiri for pointing this out!! Updated the PR. Now working on writing the test for delivery tag accessor and cleaning the PR :)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615454611
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,115 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <iostream>
+#include <map>
+
+#include <cstdio>
+#include <cstdlib>
+#include <ctime>
+#include <sstream>
+#include <string>
+
+class direct_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ direct_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+
+ std::cout << msg.body() << std::endl;
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ }
+};
+
+class simple_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ simple_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ proton::connection_options co;
+ sender = c.open_sender(url, co);
+ }
+
+ void on_sendable(proton::sender &s) PN_CPP_OVERRIDE {
+ proton::message msg;
+ std::string m = "testing";
+ msg.body(m);
+ s.send(msg);
+ }
+};
+
+int test_delivery() {
+ std::string address("127.0.0.1:5672/examples");
+ direct_recv recv(address);
+ proton::container(recv).run();
Review comment:
I tried using the first approach as you suggested i.e. to start the receiver in a new thread. In addition, I had to use `condition_variable` just to make sure that the listener thread is ready before the sender sends the message. It seems to be working fine now. Thanks!!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616130600
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
https://github.com/apache/qpid-proton/blob/8ddf5399013b02f1fd13bda3fee7886bd48a98be/cpp/src/messaging_adapter.cpp#L118-L144
The execution goes through line 133 there, so it misses the only call to `on_delivery_settle` in the entire codebase below in the else if branch. There's no easy way to get `on_delivery_settle` called that I can see, so please remove it again. Sorry, bad advice the first time.
BTW, good idea using the coverage to see what's happening. Do you have a debugger too? My first thought would be to go for a debugger; it does not give such a persuasive screenshot that you've provided, but on the other hand it actually tells you what's happening line after line...
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r611927885
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
Yeah, that's the one I saw too.
```
cannot convert ‘proton::internal::wrapped<proton::tag>::type’ {aka ‘pn_bytes_t’} to ‘proton::internal::wrapped<proton::tag>::type*’ {aka ‘pn_bytes_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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-821153641
> My guess: you created a new file tag.hpp and forgot to commit it?
I once created this file but later deleted it, then forgot to delete it from include. :'(
> @DreamPearl It's probably a high time you fixed the CI failure https://github.com/apache/qpid-proton/pull/309/checks?check_run_id=2362084471#step:11:185
I am still working on fixing tests.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek edited a comment on pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek edited a comment on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-824691409
@DreamPearl Now close the Jira, PROTON-2370. It should have yourself as assignee, the next upcoming release as the Fix Version and status should be **Resolved** (not Closed, that will be set upon release).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r618229224
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
Okay. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r630475833
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
For future reference, the issue for `tracker` update is https://issues.apache.org/jira/browse/PROTON-2382.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615803509
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
Fixed the `ThreadSanitizer warning` and added the `on_delivery_settle()` method. I was thinking of adding the tracker methods in a new PR.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
astitcher commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r617735700
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
}
-tracker sender::send(const message &message) {
- uint64_t id = ++tag_counter;
- pn_delivery_t *dlv =
- pn_delivery(pn_object(), pn_dtag(reinterpret_cast<const char*>(&id), sizeof(id)));
+tracker sender::send(const message &message, const binary &tag) {
Review comment:
Lgtm now
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615455111
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,115 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <iostream>
+#include <map>
+
+#include <cstdio>
+#include <cstdlib>
+#include <ctime>
+#include <sstream>
+#include <string>
+
+class direct_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ direct_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+
+ std::cout << msg.body() << std::endl;
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ }
+};
+
+class simple_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ simple_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ proton::connection_options co;
+ sender = c.open_sender(url, co);
+ }
+
+ void on_sendable(proton::sender &s) PN_CPP_OVERRIDE {
+ proton::message msg;
+ std::string m = "testing";
+ msg.body(m);
+ s.send(msg);
+ }
+};
+
+int test_delivery() {
+ std::string address("127.0.0.1:5672/examples");
+ direct_recv recv(address);
+ proton::container(recv).run();
+ simple_send send(address);
+ proton::container(send).run();
+
+ return 0;
+}
+
+int main(int argc, char **argv) {
+ int failed = 0;
+ RUN_ARGV_TEST(failed, test_delivery());
Review comment:
Thanks, I understand.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] gemmellr commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r618517799
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
For later reference, the new JIRA is https://issues.apache.org/jira/browse/PROTON-2377
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616484981
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
Thread safe is a matter of degree, of course. The situation I am thinking about is program with two threads, each running its own container and not interacting in any way with the other thread. I believed this is something that Proton supports or at least strives to support. In this case, because `uint64_t tag_counter = 0;` is a global variable, the two threads will step over each other's toes anyways and will send duplicate tags.
My suggestion for Raki here was to create a Jira ticket for it. Do you think what's happening is completely expected and raising Jira is not warranted? I fully agree that the issue has low priority. It has been this way for ages and noone ever complained.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r612580842
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
@ssorj I think I now see what you meant, the `pn_delivery_tag` is a type alias for `pn_bytes_t`, and the reasonable types for it in C++ are either std::string or std::vector<uint8> (or Proton's `binary`). So, one of the two following functions should be used instead of `make_wrapper`:
https://github.com/apache/qpid-proton/blob/8ddf5399013b02f1fd13bda3fee7886bd48a98be/cpp/src/types_internal.hpp#L58-L59
Both make a copy of the data, which simplifies things.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r618232507
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
Yes, It's working but I tested it on a sample code only. Still figuring out how to use it for qpid proton.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615805220
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
Thanks, I will take a look.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615804137
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,115 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <iostream>
+#include <map>
+
+#include <cstdio>
+#include <cstdlib>
+#include <ctime>
+#include <sstream>
+#include <string>
+
+class direct_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ std::cout << "listening on " << l.port() << std::endl;
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ direct_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+
+ std::cout << msg.body() << std::endl;
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ }
+};
+
+class simple_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ simple_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ proton::connection_options co;
+ sender = c.open_sender(url, co);
+ }
+
+ void on_sendable(proton::sender &s) PN_CPP_OVERRIDE {
+ proton::message msg;
+ std::string m = "testing";
+ msg.body(m);
+ s.send(msg);
+ }
+};
+
+int test_delivery() {
+ std::string address("127.0.0.1:5672/examples");
+ direct_recv recv(address);
+ proton::container(recv).run();
Review comment:
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek merged pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek merged pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r611818315
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
@jiridanek I added an additional template in `cpp/src/proton_bits.hpp` and the error went away. Now I am able to build but I am not sure if we can do it like this, WDYT?
https://github.com/apache/qpid-proton/pull/309/files#diff-eefc5fa01e2b6566e1ae3b0034ba358f7f7bd0530d5dd81986cf64a77ce276f3R159
Meanwhile, I am trying writing tests so that we can know if it is working fine or not.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
astitcher commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616843329
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
tag_counter is effectively a global and not used in a thread safe way. This is the libraries responsibility not the user application. So there is a clear threading bug here in the sender logic.
The underlying C proton library is "thread neutral" as long as you process whole connections in a thread serialised way. However this piece of the C++ library is clearly at best racy giving duplicate ids out to different threads and at worst a bug risk. We can fix it by using C++ atomics now that we can use C++11.
##########
File path: cpp/include/proton/sender.hpp
##########
@@ -55,7 +55,7 @@ PN_CPP_CLASS_EXTERN sender : public link {
PN_CPP_EXTERN void open(const sender_options &opts);
/// Send a message on the sender.
- PN_CPP_EXTERN tracker send(const message &m);
+ PN_CPP_EXTERN tracker send(const message &m, const binary &tag = binary());
Review comment:
It would actually be better to add a new signature for ```sender::send()```: This is because of considerations of ABI compatibility.
When you change the signature by adding new parameters to a function you change the 'mangled' symbol name that the compiler uses to ensure that programs get the correct overload of a function when linking the program together. This means that any existing compiled program (that is not recompiled) will not find the symbol for ```sender::send()``` that it was originally linked with.
So in this case to ensure that existing programs continue to work and not to cause an ABI breakage it would be better to introduce a new ```tracker send(const message &m, const binary &tag)``` overload. Note that this needs to be without the defaulted tag parameter for this to work. In the actual implementations you can have one implementation call the other to avoid cut-n-paste programming!
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
}
-tracker sender::send(const message &message) {
- uint64_t id = ++tag_counter;
- pn_delivery_t *dlv =
- pn_delivery(pn_object(), pn_dtag(reinterpret_cast<const char*>(&id), sizeof(id)));
+tracker sender::send(const message &message, const binary &tag) {
Review comment:
As above, I think you should keep the original ```sender::send(const message&)``. There are a few ways to make sure that both functions share most of the code rather than cutting and pasting the common parts - you can decide how to do it.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r617746897
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
@DreamPearl please fill a jira then, and close this thread. Thanks!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] codecov-commenter edited a comment on pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-822450129
# [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#309](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fcfb95b) into [main](https://codecov.io/gh/apache/qpid-proton/commit/19ae83a1ef3ecf34c34aae72f2a6a878f138749c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (19ae83a) will **increase** coverage by `8.49%`.
> The diff coverage is `n/a`.
> :exclamation: Current head fcfb95b differs from pull request most recent head 75f7466. Consider uploading reports for the commit 75f7466 to get more accurate results
[](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## main #309 +/- ##
==========================================
+ Coverage 69.03% 77.53% +8.49%
==========================================
Files 351 46 -305
Lines 69481 2341 -67140
==========================================
- Hits 47967 1815 -46152
+ Misses 21514 526 -20988
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [ruby/lib/handler/reactor\_messaging\_adapter.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvaGFuZGxlci9yZWFjdG9yX21lc3NhZ2luZ19hZGFwdGVyLnJi) | `32.55% <0.00%> (-58.14%)` | :arrow_down: |
| [ruby/lib/codec/data.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29kZWMvZGF0YS5yYg==) | `57.82% <0.00%> (-38.27%)` | :arrow_down: |
| [ruby/lib/core/delivery.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS9kZWxpdmVyeS5yYg==) | `57.57% <0.00%> (-36.37%)` | :arrow_down: |
| [ruby/lib/handler/messaging\_handler.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvaGFuZGxlci9tZXNzYWdpbmdfaGFuZGxlci5yYg==) | `57.14% <0.00%> (-35.72%)` | :arrow_down: |
| [ruby/lib/core/tracker.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS90cmFja2VyLnJi) | `55.55% <0.00%> (-33.34%)` | :arrow_down: |
| [ruby/lib/core/message.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS9tZXNzYWdlLnJi) | `57.05% <0.00%> (-31.77%)` | :arrow_down: |
| [ruby/lib/util/deprecation.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvdXRpbC9kZXByZWNhdGlvbi5yYg==) | `70.58% <0.00%> (-29.42%)` | :arrow_down: |
| [ruby/lib/util/error\_handler.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvdXRpbC9lcnJvcl9oYW5kbGVyLnJi) | `59.37% <0.00%> (-28.13%)` | :arrow_down: |
| [ruby/lib/core/uri.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS91cmkucmI=) | `74.07% <0.00%> (-25.93%)` | :arrow_down: |
| [ruby/lib/core/condition.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS9jb25kaXRpb24ucmI=) | `80.64% <0.00%> (-19.36%)` | :arrow_down: |
| ... and [306 more](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [19ae83a...75f7466](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r616151542
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
> No, I haven't used it. Will look into it.
OK, let me give you a few pointers. If you already use an IDE, you should use something built-in into it. If you use e.g. VS Code, there should be a debugger for C++ as a plugin. It is good to use a debugger in a program you already know how to use. Learning a new IDE at the same time makes things hard. What IDE/editor are you using for C++?
Here's a tutorial for VS Code. Looking at it, I feel QT Creator was simpler ;P https://www.youtube.com/watch?v=G9gnSGKYIg4
When I had C++ at school, we used the free QT Creator IDE which was quite nice, or Clion is ok, if you have a Clion licence from your school (which we had). Another possibility is Eclipse for C/C++ which @ganeshmurthy is probably still using.
There is a command-line debugger GDB (and LLDB, which looks essentially the same). Pretty much all the other graphical debuggers are simply graphical front ends for GDB. I do not recommend using GDB directly when you have another option.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615869449
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
did you step through the code in debugger? I think you stop the container by doing these three things; does the ASSERT_EQUAL that's after this even run?
since you've added `on_delivery_settle`, I think you should stop the receiver there; that should be the last event callback that gets called, unless I am mistaken
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r619689825
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
Awesome. As a further improvement you could look into, there's probably something like these .env files, https://github.com/microsoft/vscode-python/issues/544. The idea is that you don't have to specify env variables in the launch config, which is quite clumsy, but instead you can keep them in a more readable format in a file. But I guess there was not too many environment variables necessary. Probably just that `LD_LIBRARY_PATH` I mentioned, right?
If you want some more work now, and to practice debugging ;P you could have a look at https://issues.apache.org/jira/browse/PROTON-2104. (Unless Justin finds you something better, or it's already time to look into your main task.) If you recall your tests for the url class, there were some tests for IPv6, and they were passing. So, how it's possible that IPv6 connections don't work? I tried stepping through the code in a debugger some time ago, and I left a comment. You'd have to double-check if the observations are still valid. Use the step-into command to follow the program execution, and see where the url string breaks. Then, you can propose a solution, where to best apply a fix.
## Motivational cartoon, just for fun ;)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r617699833
##########
File path: cpp/include/proton/sender.hpp
##########
@@ -55,7 +55,7 @@ PN_CPP_CLASS_EXTERN sender : public link {
PN_CPP_EXTERN void open(const sender_options &opts);
/// Send a message on the sender.
- PN_CPP_EXTERN tracker send(const message &m);
+ PN_CPP_EXTERN tracker send(const message &m, const binary &tag = binary());
Review comment:
Thanks for explaining! Fixed it.
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
}
-tracker sender::send(const message &message) {
- uint64_t id = ++tag_counter;
- pn_delivery_t *dlv =
- pn_delivery(pn_object(), pn_dtag(reinterpret_cast<const char*>(&id), sizeof(id)));
+tracker sender::send(const message &message, const binary &tag) {
Review comment:
Thanks! I tried something, can you please take a look?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r613505096
##########
File path: cpp/src/delivery.cpp
##########
@@ -40,6 +42,7 @@ namespace proton {
delivery::delivery(pn_delivery_t* d): transfer(make_wrapper(d)) {}
receiver delivery::receiver() const { return make_wrapper<class receiver>(pn_delivery_link(pn_object())); }
+tag delivery::tag() const { return make_wrapper<class tag>(pn_delivery_tag(pn_object())); }
Review comment:
@jiridanek @gemmellr Thanks for the explanation.
Fow now added the default delivery tag argument in `sender::send()`
`PN_CPP_EXTERN tracker send(const message &m,const binary &tag = binary());`
WDYT?
Meanwhile working on writing tests and code formatting.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-821132679
@DreamPearl It's probably a high time you fixed the CI failure https://github.com/apache/qpid-proton/pull/309/checks?check_run_id=2362084471#step:11:185
```
/home/runner/work/qpid-proton/qpid-proton/cpp/src/delivery.cpp:26:10: fatal error: proton/tag.hpp: No such file or directory
26 | #include "proton/tag.hpp"
| ^~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [cpp/CMakeFiles/qpid-proton-cpp.dir/build.make:244: cpp/CMakeFiles/qpid-proton-cpp.dir/src/delivery.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:2274: cpp/CMakeFiles/qpid-proton-cpp.dir/all] Error 2
make: *** [Makefile:146: all] Error 2
Error: Process completed with exit code 2.
```
My guess: you created a new file tag.hpp and forgot to commit it?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r618251505
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
Here's what I'd do. `ctest -VV -N -R delivery_test` will give you command that ctest uses to run test. You'd need to write this into the VSCode launch configuration. Some of the env variables are not important, and some are. LD_LIBRARY_PATH is important, for example. Best to find a minimal command that will work (in commandline) and then transfer that to the launch configuration.
If you want to run an example, etc, it's the same. Find out the minimal command on a (fresh) commandline, without doing `source build/config.sh` and put that into VSC launch config.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r613514448
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,17 @@ namespace {
uint64_t tag_counter = 0;
}
-tracker sender::send(const message &message) {
+tracker sender::send(const message &message,const binary &tag) {
+ pn_delivery_t *dlv;
+ if(!tag.empty())
Review comment:
Try to create `.clang-format` file containing this
```
Language: Cpp # also applies to C
BasedOnStyle: LLVM
IndentWidth: 4
AllowShortIfStatementsOnASingleLine: true
AllowShortLoopsOnASingleLine: true
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r613512616
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,17 @@ namespace {
uint64_t tag_counter = 0;
}
-tracker sender::send(const message &message) {
+tracker sender::send(const message &message,const binary &tag) {
+ pn_delivery_t *dlv;
+ if(!tag.empty())
+ {
+ dlv = pn_delivery(pn_object(), pn_dtag((std::string(tag)).c_str(), tag.size()));
Review comment:
This could be done in a nicer way. `binary` is a vector; you are guaranteed that the individual values will be in memory one after another, so even something like `(char *) &tag[0]` should work, no need to convert to string, then do c_str.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r617722938
##########
File path: cpp/src/sender.cpp
##########
@@ -66,8 +66,15 @@ uint64_t tag_counter = 0;
tracker sender::send(const message &message) {
uint64_t id = ++tag_counter;
- pn_delivery_t *dlv =
- pn_delivery(pn_object(), pn_dtag(reinterpret_cast<const char*>(&id), sizeof(id)));
+ const binary tag(
+ std::string(reinterpret_cast<const char *>(&id), sizeof(id)));
Review comment:
Oh okay!! Thank you. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] codecov-commenter commented on pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#issuecomment-822450129
# [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#309](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f2ed11e) into [main](https://codecov.io/gh/apache/qpid-proton/commit/19ae83a1ef3ecf34c34aae72f2a6a878f138749c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (19ae83a) will **increase** coverage by `15.03%`.
> The diff coverage is `n/a`.
> :exclamation: Current head f2ed11e differs from pull request most recent head c67ba09. Consider uploading reports for the commit c67ba09 to get more accurate results
[](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## main #309 +/- ##
===========================================
+ Coverage 69.03% 84.06% +15.03%
===========================================
Files 351 46 -305
Lines 69481 2341 -67140
===========================================
- Hits 47967 1968 -45999
+ Misses 21514 373 -21141
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [ruby/lib/core/message.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS9tZXNzYWdlLnJi) | `57.05% <0.00%> (-31.77%)` | :arrow_down: |
| [ruby/lib/util/error\_handler.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvdXRpbC9lcnJvcl9oYW5kbGVyLnJi) | `59.37% <0.00%> (-28.13%)` | :arrow_down: |
| [ruby/lib/codec/data.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29kZWMvZGF0YS5yYg==) | `76.52% <0.00%> (-19.57%)` | :arrow_down: |
| [ruby/lib/handler/messaging\_adapter.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvaGFuZGxlci9tZXNzYWdpbmdfYWRhcHRlci5yYg==) | `91.30% <0.00%> (-5.80%)` | :arrow_down: |
| [ruby/lib/codec/mapping.rb](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29kZWMvbWFwcGluZy5yYg==) | `88.17% <0.00%> (-1.08%)` | :arrow_down: |
| [c/tests/test\_main.cpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Yy90ZXN0cy90ZXN0X21haW4uY3Bw) | | |
| [python/tests/proton\_tests/url.py](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHl0aG9uL3Rlc3RzL3Byb3Rvbl90ZXN0cy91cmwucHk=) | | |
| [cpp/examples/selected\_recv.cpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL2V4YW1wbGVzL3NlbGVjdGVkX3JlY3YuY3Bw) | | |
| [cpp/examples/simple\_recv.cpp](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL2V4YW1wbGVzL3NpbXBsZV9yZWN2LmNwcA==) | | |
| [build/c/src/core/memory.c](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVpbGQvYy9zcmMvY29yZS9tZW1vcnkuYw==) | | |
| ... and [294 more](https://codecov.io/gh/apache/qpid-proton/pull/309/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [19ae83a...c67ba09](https://codecov.io/gh/apache/qpid-proton/pull/309?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r617747556
##########
File path: cpp/src/delivery_test.cpp
##########
@@ -0,0 +1,139 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/container.hpp>
+#include <proton/delivery.h>
+#include <proton/delivery.hpp>
+#include <proton/link.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/message.hpp>
+#include <proton/message_id.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/tracker.hpp>
+#include <proton/types.h>
+#include <proton/types.hpp>
+#include <proton/value.hpp>
+
+#include "proton/error_condition.hpp"
+#include "proton/internal/pn_unique_ptr.hpp"
+#include "proton/receiver_options.hpp"
+#include "proton/transport.hpp"
+#include "proton/work_queue.hpp"
+#include "test_bits.hpp"
+
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) PN_CPP_OVERRIDE {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) PN_CPP_OVERRIDE {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) PN_CPP_OVERRIDE {
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
Review comment:
Is the VS Code debugger working for you?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #309: [WIP] PROTON-2370: [cpp] An accessor for the delivery tag
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #309:
URL: https://github.com/apache/qpid-proton/pull/309#discussion_r615219442
##########
File path: cpp/src/sender.cpp
##########
@@ -64,10 +64,18 @@ namespace {
uint64_t tag_counter = 0;
Review comment:
AFAIK thread safety is required, to guarantee unique tags. Or a different way of generating them should be found. Something for later, to investigate and fix.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org