[Boost-bugs] [Boost C++ Libraries] #11850: templated overload of boost::lockfree::spsc_queue::pop is a potential footgun

Subject: [Boost-bugs] [Boost C++ Libraries] #11850: templated overload of boost::lockfree::spsc_queue::pop is a potential footgun
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2015-12-17 03:11:24

#11850: templated overload of boost::lockfree::spsc_queue::pop is a potential
footgun
------------------------------+--------------------------
 Reporter:  tjakubowski@…     |      Owner:  timblechmann
     Type:  Feature Requests  |     Status:  new
Milestone:  To Be Determined  |  Component:  lockfree
  Version:                    |   Severity:  Problem
 Keywords:                    |
------------------------------+--------------------------
 boost::lockfree::spsc_queue::pop has many overloads. I've seen code that
 to tries to select this one:

 {{{
 template<typename U>
   boost::enable_if< typename is_convertible< T, U >::type, bool >::type
   pop(U & ret);
 }}}

 but, by mistake, selects this one:

 {{{
 template<typename OutputIterator>
   boost::disable_if< typename is_convertible< T, OutputIterator >::type,
 size_type >::type
   pop(OutputIterator it);
 }}}

 How? The problematic code looks like this:

 {{{
 boost::spsc_queue<Thing> q;

 void consumer()
 {
   for (;;) {
     while (!q.empty()) {
       OtherThing foo = ...
       Thing bar;
       AnotherThing baz = ...;
       q.pop(&bar);
       // do something with foo, bar, and baz
     }
   }
 }
 }}}

 Note the & operator, which will select the templated "pop to an output
 iterator" overload because pointers to non-const are output iterators,
 rather than lead to a compilation error (as one might expect). This is a
 nasty bug because when there isn't much pressure on the queue, and at most
 one element is present, the consumer will appear to work correctly. But if
 there is more than one element in the queue, popping all of them off will
 cause undefined behavior (probably overwriting the memory of foo or baz in
 the above example) that, speaking from experience, is pretty hard to nail
 down.

 My request is to deprecate this templated "pop to an iterator" overload,
 and make a new member function called pop_all with the same signature to
 replace it. This would help to prevent this mistake, and also make the API
 somewhat more consistent with the consume_one/consume_all functions.

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/11850>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.