Re: [Boost-users] Interprocess container segmentation fault (example provided)

Subject: Re: [Boost-users] Interprocess container segmentation fault (example provided)
From: Anthony Foiani (tkil_at_[hidden])
Date: 2012-03-02 03:33:13

Josh --

"Davidson, Josh" <josh.davidson_at_[hidden]> writes:

> Since the map itself a member of TestStore and TestStore is being
> constructed from the segment, I would think that should be roughly
> equivalent to just creating the map by itself from the segment.
> However, as you'll see if you run it, it still seg faults when
> accessing the list iterator.

I believe that Ion's comments about the use of 'm' in
TestStore::addMessage are still correct and are near the root of the
problem.

I'd recommend arranging your class so that successful construction
implies:

1. the named shared segment itself has been found or created and
   mapped;

2. the named object within the segment has been found or created;

3. the object is in a valid state and ready for operations.

Additionally, you should treat this as a Singleton within each process
(to avoid the multiple-mapping problems that Ion mentioned).  You can
do that with simple discipline / review, or you can force it by hiding
it behind a factory method, employing "boost::noncopyable", etc.

This means that your "addMessage" method will always have exactly the
correct segment manager to allocate additional resources.

I wrote a very similar container, although it only mapped strings to
strings:

   http://scrye.com/~tkil/cpp/boost-interprocess/SharedMap.hpp
   http://scrye.com/~tkil/cpp/boost-interprocess/SharedMap.cpp
   http://scrye.com/~tkil/cpp/boost-interprocess/SharedMapTest.cpp

(The test won't compile, as it relies on other classes from the
original codebase; but it should show you how the SharedMap class is
intended to be used.)

I also wrote a few replies on the list, a month or two back, that
touch on these same topics:

   http://groups.google.com/group/boost-list/msg/ee619a75277b2146

My version of your code follows; it is also available at:

  http://scrye.com/~tkil/cpp/boost-interprocess/josh-shared-data.cpp

The main changes are:

a. Make all the typedefs private to the class; they're an
   implementation detail.

b. Have the class do all the work of lookup, mapping, and allocation;
   this makes for a much simpler interface.

c. Minor issues: within the class, there is only one Map, List, and
   Datum type, so remove some of the very long prefixes.

And a few other bits.  Hopefully you'll find this useful.

Best regards,
Anthony Foiani

========================================================================
// linux compile line:
//   g++ -O2 -o josh-shared-data josh-shared-data.cpp -lpthread -lrt

#include <stdint.h>
#include <string>
#include <iostream>
#include <utility>

#include <boost/foreach.hpp>
#include <boost/interprocess/allocators/allocator.hpp>
#include <boost/interprocess/containers/list.hpp>
#include <boost/interprocess/containers/map.hpp>
#include <boost/interprocess/managed_shared_memory.hpp>

namespace bi = boost::interprocess;

class TestStore
{

public:

    struct Datum {
        uint16_t commandStatus;
        std::size_t wordCount;
        uint16_t data[32];

        Datum( uint16_t commandStatus, std::size_t wordCount )
            : commandStatus( commandStatus ),
              wordCount( wordCount )
        {
            for ( std::size_t i = 0; i < 32; ++i )
                data[i] = 0;
        }

        friend std::ostream & operator << ( std::ostream & os,
                                            const Datum & data )
        {
            os << "[Datum: cs=" << data.commandStatus
               << ", wc=" << data.wordCount
               << ", data=<" << data.data[0];
            for ( std::size_t i = 1; i < 32; ++i )
                os << " " << data.data[i];
            os << ">]";
            return os;
        }
    };

    enum ReplaceFlag
    {
        REPLACE_EXISTING,
        KEEP_EXISTING
    };

    TestStore( const std::string & segName,
               const std::string & mapName,
               const ReplaceFlag replace = KEEP_EXISTING,
               const std::size_t segSize = 600U*1024U )
        : mReplacer( segName, replace ),
          mSegment( bi::open_or_create, segName.c_str(), segSize ),
          mData( mSegment.find_or_construct< Map >( mapName.c_str() )(
                     std::less< Key >(),
                     mSegment.get_allocator< MapValueType >() ) )
    {
    }

    void printData()
    {
        using std::clog;
        using std::endl;

        BOOST_FOREACH( const Map::value_type & outer, *mData )
        {
            clog << "key: " << outer.first << endl;
            BOOST_FOREACH( const List::value_type & inner, outer.second )
              clog << "  " << inner << endl;
        }
    }

    bool addMessage( unsigned key, const Datum & datum )
    {
        Map::iterator it( mData->find(key) );

        if ( it == mData->end() )
        {
            ListAllocator alloc( mSegment.get_segment_manager() );
            std::pair< Map::iterator, bool > rv(
                mData->insert( Map::value_type( key, List( alloc ) ) )
            );
            it = rv.first;
        }

        it->second.push_back( datum );

        return true;
    }

private:

    typedef bi::managed_shared_memory Segment;
    typedef Segment::segment_manager Manager;
 
    typedef bi::allocator< Datum, Manager > ListAllocator;
    typedef bi::list< Datum, ListAllocator > List;

    typedef unsigned int Key;

    typedef std::pair< const Key, List > MapValueType;

    typedef bi::allocator< MapValueType, Manager > MapAllocator;
    typedef bi::map< Key, List, std::less< Key >, MapAllocator> Map;

    struct Replacer
    {
        Replacer( const std::string segName,
                  const ReplaceFlag action )
        {
            if ( action == REPLACE_EXISTING )
                bi::shared_memory_object::remove( segName.c_str() );
        }
    };

    Replacer mReplacer;
    Segment mSegment;
    Map * mData;
};

int main()
{
    const std::string segName = "JoshSharedSeg";
    const std::string mapName = "map";

    TestStore store( segName, mapName, TestStore::REPLACE_EXISTING );

    using std::clog;
    using std::endl;

    clog << "before:" << endl;
    store.printData();

    TestStore::Datum datum( 10, 14 );
    store.addMessage( 3, datum );

    clog << "after:" << endl;
    store.printData();

    return 0;
}