You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2016/05/18 22:07:17 UTC

qpid-proton git commit: PROTON-1200: c++ multi-threaded and IO integration documentation.

Repository: qpid-proton
Updated Branches:
  refs/heads/master 906881bf5 -> 15108822e


PROTON-1200: c++ multi-threaded and IO integration documentation.

- updated documentation
- removed/privatized unused functions in thread_safe.hpp


Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/15108822
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/15108822
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/15108822

Branch: refs/heads/master
Commit: 15108822e71699826b8ec84eabe5314fc8dd6ce5
Parents: 906881b
Author: Alan Conway <ac...@redhat.com>
Authored: Wed May 18 16:59:03 2016 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Wed May 18 18:04:21 2016 -0400

----------------------------------------------------------------------
 examples/cpp/README.dox                         |  4 +-
 examples/cpp/mt/broker.cpp                      | 21 +++---
 examples/cpp/mt/epoll_container.cpp             |  1 -
 examples/cpp/tutorial.dox                       |  2 +-
 proton-c/bindings/cpp/docs/io.md                | 24 +++----
 proton-c/bindings/cpp/docs/mt.md                | 54 ++++++++++------
 .../bindings/cpp/include/proton/container.hpp   | 24 +++----
 .../cpp/include/proton/default_container.hpp    | 16 ++---
 .../bindings/cpp/include/proton/event_loop.hpp  | 26 ++++----
 .../cpp/include/proton/io/connection_engine.hpp |  4 +-
 .../bindings/cpp/include/proton/thread_safe.hpp | 68 ++++++++------------
 proton-c/bindings/cpp/src/container_impl.cpp    |  1 -
 12 files changed, 119 insertions(+), 126 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/examples/cpp/README.dox
----------------------------------------------------------------------
diff --git a/examples/cpp/README.dox b/examples/cpp/README.dox
index 1b7020b..897b302 100644
--- a/examples/cpp/README.dox
+++ b/examples/cpp/README.dox
@@ -133,9 +133,7 @@ __Requires C++11__
 
 /** @example mt/broker.cpp
 
-A multithreaded broker, using the proton::mt extensions. This broker is
-portable over any implementation of the proton::mt API, see @ref
-mt/epoll_container.cpp for an example.
+A multithreaded broker, that will work on any multi-threaded container. See @ref mt/epoll_container.cpp for an example of a multi-threaded container.
 
 __Requires C++11__
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/examples/cpp/mt/broker.cpp
----------------------------------------------------------------------
diff --git a/examples/cpp/mt/broker.cpp b/examples/cpp/mt/broker.cpp
index 00ebb55..29fe97f 100644
--- a/examples/cpp/mt/broker.cpp
+++ b/examples/cpp/mt/broker.cpp
@@ -120,24 +120,30 @@ class queues {
 /// concurrently. Resources used by multiple connections (e.g. the queues in
 /// this example) must be thread-safe.
 ///
-/// FIXME aconway 2016-05-10: doc - point out queue/sender interaction as
-/// example of communication via event_loop::inject()
+/// 4. You can 'inject' work to be done sequentially using a connection's
+/// proton::event_loop. In this example, we create a std::function callback
+/// that we pass to queues, so they can notify us when they have messages.
 ///
 class broker_connection_handler : public proton::messaging_handler {
   public:
     broker_connection_handler(queues& qs) : queues_(qs) {}
 
     void on_connection_open(proton::connection& c) OVERRIDE {
-        // Create the has_messages callback for use with queue subscriptions.
+        // Create the has_messages callback for queue subscriptions.
         //
-        // FIXME aconway 2016-05-09: doc lifecycle: handler tied to c.
-        // explain why this is safe & necessary
+        // Make a std::shared_ptr to a thread_safe handle for our proton::connection.
+        // The connection's proton::event_loop will remain valid as a shared_ptr exists.
         std::shared_ptr<proton::thread_safe<proton::connection> > ts_c = make_shared_thread_safe(c);
+
+        // Make a lambda function to inject a call to this->has_messages() via the proton::event_loop.
+        // The function is bound to a shared_ptr so this is safe. If the connection has already closed
+        // proton::event_loop::inject() will drop the callback.
         has_messages_callback_ = [this, ts_c](queue* q) mutable {
             ts_c->event_loop()->inject(
                 std::bind(&broker_connection_handler::has_messages, this, q));
         };
-        c.open();            // Always accept
+
+        c.open();            // Accept the connection
     }
 
     // A sender sends messages from a queue to a subscriber.
@@ -211,8 +217,7 @@ class broker_connection_handler : public proton::messaging_handler {
         return popped;
     }
 
-    // FIXME aconway 2016-05-09: doc
-    // Called via the connections event_loop when q has messages.
+    // Called via the connection's proton::event_loop when q has messages.
     // Try all the blocked senders.
     void has_messages(queue* q) {
         auto range = blocked_.equal_range(q);

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/examples/cpp/mt/epoll_container.cpp
----------------------------------------------------------------------
diff --git a/examples/cpp/mt/epoll_container.cpp b/examples/cpp/mt/epoll_container.cpp
index 7f9fb86..fe516e9 100644
--- a/examples/cpp/mt/epoll_container.cpp
+++ b/examples/cpp/mt/epoll_container.cpp
@@ -104,7 +104,6 @@ class epoll_container : public proton::io::container_impl_base {
     epoll_container(const std::string& id);
     ~epoll_container();
 
-    // Implemenet the proton::mt_container interface
     proton::returned<proton::connection> connect(
         const std::string& addr, const proton::connection_options& opts) OVERRIDE;
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/examples/cpp/tutorial.dox
----------------------------------------------------------------------
diff --git a/examples/cpp/tutorial.dox b/examples/cpp/tutorial.dox
index 468c4f7..87456b9 100644
--- a/examples/cpp/tutorial.dox
+++ b/examples/cpp/tutorial.dox
@@ -20,7 +20,7 @@ how to run them using the `proton::container`, a portable, easy-to-use way to
 build single-threaded clients or servers.
 
 @note Proton can also be embedded or integrated into arbitrary IO frameworks and used
-to build multi-threaded applications. See namespace proton::mt for more.
+to build multi-threaded applications. See @ref mt_page for more.
 
 Some of the examples require an AMQP *broker* that can receive, store and send
 messages. @ref broker.hpp and @ref broker.cpp define a simple example

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/proton-c/bindings/cpp/docs/io.md
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/docs/io.md b/proton-c/bindings/cpp/docs/io.md
index e6ee386..0eac8da 100644
--- a/proton-c/bindings/cpp/docs/io.md
+++ b/proton-c/bindings/cpp/docs/io.md
@@ -2,20 +2,20 @@
 
 **Experimental**
 
-The proton::io namespace contains a low-level service provider
-interface (SPI) that can be used to implement the proton API over any
-native or third-party IO library.
+The `proton::io` namespace contains a service provider interface (SPI) that
+allows you to implement the @ref proton API over alternative IO or threading
+libraries.
 
-The proton::io::connection_engine is the core engine that converts raw
-AMQP bytes read from any IO source into proton::messaging_handler
-event calls and generates AMQP byte-encoded output that can be written
-to any IO destination.
+The `proton::io::connection_engine` converts an AMQP-encoded byte stream, read
+from any IO source, into `proton::messaging_handler` calls. It generates an
+AMQP-encoded byte stream as output, that can be written to any IO destination.
 
-Integrations need to implement two user-visible interfaces:
+The connection_engine can be used stand-alone, as an AMQP translator or you
+can implement the following two interfaces to provide a complete implementation
+of the proton API, that can run any proton application:
 
- - proton::container lets the user initiate or listen for connections.
+ - `proton::container` lets the user initiate or listen for connections.
 
- - proton::event_loop lets the user serialize their own work with a
-   connection.
+ - `proton::event_loop` lets the user serialize work with a connection.
 
-@see @ref mt/epoll\_container.cpp for an example of an integration.
+@see @ref mt/epoll\_container.cpp for an example.

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/proton-c/bindings/cpp/docs/mt.md
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/docs/mt.md b/proton-c/bindings/cpp/docs/mt.md
index 73239e5..7c3cf32 100644
--- a/proton-c/bindings/cpp/docs/mt.md
+++ b/proton-c/bindings/cpp/docs/mt.md
@@ -1,29 +1,41 @@
-# Multithreaded Proton {#mt_page}
+# Multithreaded Proton Applications {#mt_page}
 
 **Experimental**
 
-<!-- FIXME aconway 2016-05-04: doc -->
+For an example see @ref mt/broker.cpp
 
-Most classes in namespace @ref proton are not thread-safe. Objects
-associated with a single connection *must not* be used
-concurrently. However, objects associated with *different* connections
-*can* be used concurrently in separate threads.
+Most classes in namespace @ref proton are not thread-safe. Objects belonging
+to a single connection (`proton::connection`, `proton::sender`,
+`proton::receiver` and so on) *must not* be used concurrently. Objects associated with
+*different* connections *can* be used concurrently in separate threads.
 
-The recommended way to use proton multithreaded is to *serialize* the
-work for each connection but allow different connections to be
-processed concurrently.
+A multi-threaded container calls event-handling functions for each connection
+*sequentially* but can process *different* connections concurrently in different
+threads. If you use a *separate* `proton::messaging_handler` to for each
+connection, then event handling functions can can use their parameters and the
+handler's own data members without locks. The handler functions will never be
+called concurrently. You can set the handlers for each connection using
+`proton::container::connect()` and `proton::container::listen()`.
 
-proton::container allows you to manage connections in a multithreaded
-way. You supply a proton::messaging_handler for each
-connection. Proton will ensure that the
-`proton::messaging_handler::on_*()` functions are never called
-concurrently so per-connection handlers do not need a lock even if
-they have state.
+The example @ref mt/broker.cpp is a multi-threaded broker using this approach.
+It creates a new handler for each incoming connection to manage the state of
+that connection's `proton::sender` and `proton::receiver` links. The handler
+needs no lock because it only deals with state for one connection.
 
-proton::event_loop allows you to make calls to arbitrary functions or
-other code, serialized in the same way as
-`proton::messaging_handler::on_*()` calls. Typically this is used to
-call your own handler's member functions in the same way as
-proton::messaging_handler override functions.
+The `proton::event_loop` represents the sequence of events associated with a
+connection.  `proton::event_loop::inject()` allows another thread to "inject"
+work to be executed in sequence with the rest of the events so it can operate
+safely on data associated with the connection.
 
-For an example see @ref mt/broker.cpp.
+In the @ref mt/broker.cpp example, a queue can receive messages from one
+connection but have subscribers on another connection. Subscribers pass a
+function object to the queue which uses `proton::event_loop::inject()` to call a
+notification callback on the handler for that connection. The callback is
+executed in the connection's event-loop so it can use a `proton::sender` object
+to send the message safely.
+
+Note: It is possible to share a single handler between more than one connection.
+In that case it *can* be called concurrently on behalf of different connections, so
+you will need suitable locking.
+
+@see @ref io_page - implementing your own container.

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/proton-c/bindings/cpp/include/proton/container.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/container.hpp b/proton-c/bindings/cpp/include/proton/container.hpp
index ce5710d..01fba85 100644
--- a/proton-c/bindings/cpp/include/proton/container.hpp
+++ b/proton-c/bindings/cpp/include/proton/container.hpp
@@ -22,8 +22,6 @@
  *
  */
 
-// FIXME aconway 2016-05-04: doc
-
 #include "./connection_options.hpp"
 #include "./error_condition.hpp"
 #include "./listener.hpp"
@@ -84,8 +82,6 @@ class PN_CPP_CLASS_EXTERN container {
     virtual void stop_listening(const std::string& url) = 0;
     /// @endcond
 
-    // FIXME aconway 2016-05-13: doc options
-
     /// Start listening on url.
     ///
     /// Calls to the @ref listen_handler are serialized for this listener,
@@ -105,17 +101,16 @@ class PN_CPP_CLASS_EXTERN container {
     PN_CPP_EXTERN virtual listener listen(const std::string& url);
 
     /// Run the container in this thread.
-    /// Returns when the container stops: see auto_stop() and stop().
+    /// Returns when the container stops.
+    /// @see auto_stop() and stop().
     ///
     /// With a multithreaded container, call run() in multiple threads to create a thread pool.
     virtual void run() = 0;
 
-    /// If true, the container will stop (i.e., run() will return)
-    /// when all active connections and listeners are closed. If false
-    /// the container will keep running till stop() is called.
+    /// If true, stop the container when all active connections and listeners are closed.
+    /// If false the container will keep running till stop() is called.
     ///
     /// auto_stop is set by default when a new container is created.
-    // FIXME aconway 2016-05-06: doc
     virtual void auto_stop(bool) = 0;
 
     /// Stop the container with an error_condition err.
@@ -126,7 +121,8 @@ class PN_CPP_CLASS_EXTERN container {
     ///  - run() will return in all threads.
     virtual void stop(const error_condition& err) = 0;
 
-    /// Stop the container with an empty error condition. See stop(const error_condition&)
+    /// Stop the container with an empty error condition.
+    /// @see stop(const error_condition&)
     PN_CPP_EXTERN virtual void stop();
 
     /// Open a connection to `url` and open a sender for `url.path()`.
@@ -155,7 +151,7 @@ class PN_CPP_CLASS_EXTERN container {
     /// container's template options.
     PN_CPP_EXTERN virtual returned<receiver> open_receiver(const std::string&url,
                                                            const proton::receiver_options &o);
-    
+
     /// Open a connection to `url` and open a receiver for
     /// `url.path()`.  Any supplied receiver or connection options will
     /// override the container's template options.
@@ -166,8 +162,6 @@ class PN_CPP_CLASS_EXTERN container {
     /// A unique identifier for the container.
     virtual std::string id() const = 0;
 
-    // FIXME aconway 2016-05-04: need timed injection to replace schedule()
-
     /// Connection options that will be to outgoing connections. These
     /// are applied first and overriden by options provided in
     /// connect() and messaging_handler::on_connection_open().
@@ -188,7 +182,7 @@ class PN_CPP_CLASS_EXTERN container {
     /// Sender options applied to senders created by this
     /// container. They are applied before messaging_handler::on_sender_open()
     /// and can be overridden.
-    virtual void sender_options(const sender_options &) = 0;
+    virtual void sender_options(const class sender_options &) = 0;
 
     /// @copydoc sender_options
     virtual class sender_options sender_options() const = 0;
@@ -196,7 +190,7 @@ class PN_CPP_CLASS_EXTERN container {
     /// Receiver options applied to receivers created by this
     /// container. They are applied before messaging_handler::on_receiver_open()
     /// and can be overridden.
-    virtual void receiver_options(const receiver_options &) = 0;
+    virtual void receiver_options(const class receiver_options &) = 0;
 
     /// @copydoc receiver_options
     virtual class receiver_options receiver_options() const = 0;

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/proton-c/bindings/cpp/include/proton/default_container.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/default_container.hpp b/proton-c/bindings/cpp/include/proton/default_container.hpp
index d7f8583..da47163 100644
--- a/proton-c/bindings/cpp/include/proton/default_container.hpp
+++ b/proton-c/bindings/cpp/include/proton/default_container.hpp
@@ -22,15 +22,13 @@
  *
  */
 
-// FIXME aconway 2016-05-04: doc
-
 #include "./container.hpp"
 
 namespace proton {
 
-// FIXME aconway 2016-05-04: doc
-
-/// A single-threaded container.    
+/// A single-threaded container.
+///
+/// @copydoc container
 class PN_CPP_CLASS_EXTERN  default_container : public container {
   public:
     /// Create a default, single-threaded container with a messaging_handler.
@@ -54,8 +52,6 @@ class PN_CPP_CLASS_EXTERN  default_container : public container {
     /// Takes ownership of c.
     PN_CPP_EXTERN explicit default_container(container* c) : impl_(c) {}
 
-    // FIXME aconway 2016-05-13: @copydoc all.
-
     PN_CPP_EXTERN returned<connection> connect(const std::string& url, const connection_options &) PN_CPP_OVERRIDE;
     PN_CPP_EXTERN listener listen(const std::string& url, listen_handler& l) PN_CPP_OVERRIDE;
 
@@ -80,14 +76,16 @@ class PN_CPP_CLASS_EXTERN  default_container : public container {
         const connection_options &c) PN_CPP_OVERRIDE;
 
     PN_CPP_EXTERN std::string id() const PN_CPP_OVERRIDE;
+
     PN_CPP_EXTERN void client_connection_options(const connection_options &o) PN_CPP_OVERRIDE;
     PN_CPP_EXTERN connection_options client_connection_options() const PN_CPP_OVERRIDE;
+
     PN_CPP_EXTERN void server_connection_options(const connection_options &o) PN_CPP_OVERRIDE;
     PN_CPP_EXTERN connection_options server_connection_options() const PN_CPP_OVERRIDE;
-    /// @copydoc container::sender_options
+
     PN_CPP_EXTERN void sender_options(const class sender_options &o) PN_CPP_OVERRIDE;
     PN_CPP_EXTERN class sender_options sender_options() const PN_CPP_OVERRIDE;
-    /// @copydoc container::receiver_options
+
     PN_CPP_EXTERN void receiver_options(const class receiver_options & o) PN_CPP_OVERRIDE;
     PN_CPP_EXTERN class receiver_options receiver_options() const PN_CPP_OVERRIDE;
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/proton-c/bindings/cpp/include/proton/event_loop.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/event_loop.hpp b/proton-c/bindings/cpp/include/proton/event_loop.hpp
index 446295a..6d231b6 100644
--- a/proton-c/bindings/cpp/include/proton/event_loop.hpp
+++ b/proton-c/bindings/cpp/include/proton/event_loop.hpp
@@ -37,32 +37,36 @@ struct pn_link_t;
 
 namespace proton {
 
-// FIXME aconway 2016-05-04: doc
-
 /// **Experimental** - A handler for injected code.
 class inject_handler {
   public:
     virtual ~inject_handler() {}
 
-    // XXX I feel like the name of this isn't quite right.  The event
-    // isn't injection, it's execution.
-    /// The code is executed.
+    // XXX bad name, should be operator()() to be idiomatic and consistent with C++11.
+    /// Called when the injected code is executed.
     virtual void on_inject() = 0;
 };
 
 /// **Experimental** - A serial execution context.
+///
+/// Event handler functions associated with a single proton::connection are called in sequence.
+/// The connection's @ref event_loop allows you to "inject" extra work from any thread,
+/// and have it executed in the same sequence.
+///
 class PN_CPP_CLASS_EXTERN event_loop {
   public:
     virtual ~event_loop() {}
 
-    // FIXME aconway 2016-05-05: doc, note bool return not throw because no
-    // atomic way to determine closd status and throw during shutdown is bad.
-    /// Send code to the event loop for execution.
-    virtual bool inject(inject_handler&) = 0;
+    /// Arrange to have f() called in the event_loop's sequence: possibly
+    /// deferred, possibly in another thread.
+    ///
+    /// @return true if f() has or will be called, false if the event_loop is ended
+    /// and f() cannot be injected.
+    virtual bool inject(inject_handler& f) = 0;
 
 #if PN_CPP_HAS_CPP11
-    /// Send code to the event loop for execution.
-    virtual bool inject(std::function<void()>) = 0;
+    /// @copydoc inject(inject_handler&)
+    virtual bool inject(std::function<void()> f) = 0;
 #endif
 
   protected:

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/proton-c/bindings/cpp/include/proton/io/connection_engine.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/io/connection_engine.hpp b/proton-c/bindings/cpp/include/proton/io/connection_engine.hpp
index 62c0230..32db158 100644
--- a/proton-c/bindings/cpp/include/proton/io/connection_engine.hpp
+++ b/proton-c/bindings/cpp/include/proton/io/connection_engine.hpp
@@ -43,8 +43,6 @@ namespace proton {
 class event_loop;
 class proton_handler;
 
-// FIXME aconway 2016-05-04: doc
-
 namespace io {
 
 class link_namer;
@@ -192,7 +190,7 @@ PN_CPP_CLASS_EXTERN connection_engine {
     connection_engine(const connection_engine&);
     connection_engine& operator=(const connection_engine&);
 
-    // FIXME aconway 2016-05-06: reduce binary compat footprint, move stuff to connection context.
+    // TODO aconway 2016-05-06: reduce binary compat footprint, move stuff to connection context.
     proton::proton_handler* handler_;
     proton::connection connection_;
     proton::transport transport_;

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/proton-c/bindings/cpp/include/proton/thread_safe.hpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/include/proton/thread_safe.hpp b/proton-c/bindings/cpp/include/proton/thread_safe.hpp
index 9925fc7..3a4ca96 100644
--- a/proton-c/bindings/cpp/include/proton/thread_safe.hpp
+++ b/proton-c/bindings/cpp/include/proton/thread_safe.hpp
@@ -30,8 +30,6 @@
 
 #include <functional>
 
-// FIXME aconway 2016-05-03: doc
-
 namespace proton {
 
 class connection;
@@ -51,21 +49,22 @@ template<> struct endpoint_traits<receiver> {};
 
 template <class T> class returned;
 
-// FIXME aconway 2016-05-09: doc
 /// **Experimental** - A thread-safe object wrapper.
-///    
-/// Events for each proton::connection are processed sequentially in
-/// an event_loop. proton::messaging_handler functions for a single connection
-/// are never called concurrently. inject() lets you add user-defined
-/// function calls to be processed in the event loop sequence.
 ///
-/// thread_safe is useful with multi-threaded programs, where
-/// different connection's event loops can run concurrently. Proton
-/// objects associated with a connection (proton::connection,
-/// proton::sender, etc.) are not thread safe, so they can only be
-/// used in the context of the connection's thread_safe.  inject()
-/// allows any thread (application threads or thread_safe threads for
-/// different connections) to communicate safely.
+/// The proton::object subclasses (proton::connection, proton::sender etc.) are
+/// reference-counted wrappers for C structs. They are not safe for concurrent use,
+/// not even to copy or assign.
+///
+/// A pointer to thread_safe<> can be used from any thread to get the
+/// proton::event_loop for the object's connection. The object will not be
+/// destroyed until the thread_safe<> is deleted. You can use std::shared_ptr,
+/// std::unique_ptr or any other memory management technique to manage the
+/// thread_safe<>.
+///
+/// Use make_thread_safe(), make_shared_thread_safe(), make_unique_thread_safe() to
+/// create a thread_safe<>
+///
+/// @see @ref mt_page
 template <class T>
 class thread_safe : private internal::pn_ptr_base, private internal::endpoint_traits<T> {
     typedef typename T::pn_type pn_type;
@@ -73,7 +72,7 @@ class thread_safe : private internal::pn_ptr_base, private internal::endpoint_tr
     struct inject_decref : public inject_handler {
         pn_type* ptr_;
         inject_decref(pn_type* p) : ptr_(p) {}
-        void on_inject() { decref(ptr_); delete this; }
+        void on_inject() PN_CPP_OVERRIDE { decref(ptr_); delete this; }
     };
 
   public:
@@ -98,23 +97,17 @@ class thread_safe : private internal::pn_ptr_base, private internal::endpoint_tr
     /// Get the event loop for this object.
     class event_loop* event_loop() { return event_loop::get(ptr()); }
 
-    /// @cond INTERNAL
-    /// XXX Not sure what the status of these is
-    
-    // FIXME aconway 2016-05-04: doc
+    /// Get the thread-unsafe proton object wrapped by this thread_safe<T>
     T unsafe() { return T(ptr()); }
 
-    // Caller must delete
-    static thread_safe* create(const T& obj) { return new (obj.pn_object()) thread_safe(); }
-
-    /// @endcond
-    
   private:
+    static thread_safe* create(const T& obj) { return new (obj.pn_object()) thread_safe(); }
     static void* operator new(size_t, pn_type* p) { return p; }
     static void operator delete(void*, pn_type*) {}
     thread_safe() { incref(ptr()); }
     pn_type* ptr() { return reinterpret_cast<pn_type*>(this); }
 
+
     // Non-copyable.
     thread_safe(const thread_safe&);
     thread_safe& operator=(const thread_safe&);
@@ -124,15 +117,18 @@ class thread_safe : private internal::pn_ptr_base, private internal::endpoint_tr
     /// @endcond
 };
 
-// FIXME aconway 2016-05-04: doc.
-// Temporary return value only, not a real smart_ptr.
-// Release or convert to some other pointer type immediately.
+// return value for functions returning a thread_safe<> object.
+//
+// Temporary return value only, you should release() to get a plain pointer or
+// assign to a smart pointer type.
 template <class T>
 class returned : private internal::endpoint_traits<T>
 {
   public:
     /// Take ownership
     explicit returned(thread_safe<T>* p) : ptr_(p) {}
+    /// Create an owned thread_safe<T>
+    explicit returned(const T& obj) : ptr_(thread_safe<T>::create(obj)) {}
     /// Transfer ownership.
     /// Use the same "cheat" as std::auto_ptr, calls x.release() even though x is const.
     returned(const returned& x) : ptr_(const_cast<returned&>(x).release()) {}
@@ -162,26 +158,16 @@ class returned : private internal::endpoint_traits<T>
     mutable thread_safe<T>* ptr_;
 };
 
-// XXX Review this text
 /// Make a thread-safe wrapper for `obj`.
-template <class T> returned<T> make_thread_safe(const T& obj) {
-    return returned<T>(thread_safe<T>::create(obj));
-}
-
-// XXX Review this text
-/// Get a thread-unsafe pointer for `p`.
-template <class T> T make_thread_unsafe(T* p) { return p->unsafe(); }
+template <class T> returned<T> make_thread_safe(const T& obj) { return returned<T>(obj); }
 
 #if PN_CPP_HAS_CPP11
 template <class T> std::shared_ptr<thread_safe<T> > make_shared_thread_safe(const T& obj) {
-    return std::shared_ptr<thread_safe<T> >(thread_safe<T>::create(obj));
+    return make_thread_safe(obj);
 }
 template <class T> std::unique_ptr<thread_safe<T> > make_unique_thread_safe(const T& obj) {
-    return std::unique_ptr<thread_safe<T> >(thread_safe<T>::create(obj));
+    return make_thread_safe(obj);
 }
-
-template <class T> T make_thread_unsafe(const std::shared_ptr<T>& p) { return p->unsafe(); }
-template <class T> T make_thread_unsafe(const std::unique_ptr<T>& p) { return p->unsafe(); }
 #endif
 
 } // proton

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/15108822/proton-c/bindings/cpp/src/container_impl.cpp
----------------------------------------------------------------------
diff --git a/proton-c/bindings/cpp/src/container_impl.cpp b/proton-c/bindings/cpp/src/container_impl.cpp
index 23734e8..b7bc9df 100644
--- a/proton-c/bindings/cpp/src/container_impl.cpp
+++ b/proton-c/bindings/cpp/src/container_impl.cpp
@@ -197,7 +197,6 @@ listener container_impl::listen(const std::string& url, listen_handler& lh) {
         throw error("already listening on " + url);
     connection_options opts = server_connection_options(); // Defaults
     proton_handler *h = opts.handler();
-    // FIXME aconway 2016-05-12: chandler and acceptor??
     internal::pn_ptr<pn_handler_t> chandler = h ? cpp_handler(h) : internal::pn_ptr<pn_handler_t>();
     proton::url u(url);
     pn_acceptor_t *acptr = pn_reactor_acceptor(


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org