1

Upon trying to write to a piece of (portable) C++ code that uses shared_memory_segment in order to write to "shared memory", I encountered boost::interprocess::bad_alloc several times. From the Boost documentation:

This exception is thrown when a memory request can't be fulfilled.

So, I must be allocating too little memory. Here follows the code (only for writing, since reading here is irrelevant):

shared_memory.h:

#include <boost/interprocess/managed_shared_memory.hpp>
#include <boost/interprocess/containers/vector.hpp>
#include <boost/interprocess/containers/string.hpp>

#include <boost/interprocess/allocators/allocator.hpp>
#include <boost/interprocess/sync/named_mutex.hpp>

#include <string>
#include <exception>

namespace my_shared_memory
{
  typedef boost::interprocess::allocator<char, boost::interprocess::managed_shared_memory::segment_manager> CharAllocator;
  typedef boost::interprocess::basic_string<char, std::char_traits<char>, CharAllocator> IPCString;
  typedef boost::interprocess::allocator<IPCString, boost::interprocess::managed_shared_memory::segment_manager> StringAllocator;
  typedef boost::interprocess::vector<IPCString, StringAllocator> ShmVector;

  bool write_to_memory(std::string wsuid, std::string loop_val, std::string should_intercept, std::string post_data) ;

  const std::string shm_prefix = "shm_";
  const std::string mutex_prefix = "mtx_";
}

shared_memory.cpp:

#include "shared_memory.h"

namespace apl_shared_memory
{

  bool write_to_memory(std::string wsuid, std::string loop_val, std::string should_intercept, std::string post_data)
  {
    bool ret_val;
    std::string shm_name = shm_prefix + wsuid;
    std::string mtx_name = mutex_prefix + wsuid;
    boost::interprocess::named_mutex named_mtx{boost::interprocess::open_or_create, mtx_name.c_str()};
    size_t size = (sizeof(loop_val) + loop_val.size() + sizeof(should_intercept) + should_intercept.size() + sizeof post_data + post_data.size()) * 5;

    try
    {
      named_mtx.lock();
      boost::interprocess::shared_memory_object::remove(shm_name.c_str());
      boost::interprocess::managed_shared_memory segment(boost::interprocess::create_only, shm_name.c_str(), size);
      CharAllocator     charallocator  (segment.get_segment_manager());
      StringAllocator   stringallocator(segment.get_segment_manager());

      IPCString shm_loop_val(charallocator);
      IPCString shm_should_intercept(charallocator);
      IPCString shm_intercepted_data(charallocator);
      shm_loop_val = loop_val.c_str();
      shm_should_intercept = should_intercept.c_str();
      shm_intercepted_data = post_data.c_str();

      segment.destroy<ShmVector>("ShmVector");
      ShmVector *shmVector = segment.construct<ShmVector>("ShmVector")(stringallocator);
      shmVector->clear();
      shmVector->push_back(shm_loop_val);
      shmVector->push_back(shm_should_intercept);
      shmVector->push_back(shm_intercepted_data);

      named_mtx.unlock();
      ret_val = true;
    } catch(const std::exception& ex)
    {
      ret_val = false;
      named_mtx.unlock();
      boost::interprocess::shared_memory_object::remove(shm_name.c_str());
    }

    named_mtx.unlock();
    return ret_val;
  }
  
}

Yes, I realize I don't need unlock calls on all three places.

Problem seems to be in the line:

size_t size = (sizeof(loop_val) + loop_val.size() + sizeof(should_intercept) + should_intercept.size() + sizeof post_data + post_data.size()) * 5;

I thought it was overkill to add times 5 but that is apparently not the case. This code works fine on Debian 9 with gcc 6.3.0 and Windows 10 with Microsoft Visual Studio 2015 Community. However, on MacOS with Xcode 10.1, I get boost::interprocess::bad_alloc when I try to insert even the first element in the vector. The solution seems to multiply the size with 10 instead of 5 but this just appears wasteful.

I added segment.get_free_memory calls and determined that for a vector that contains the following three strings (without the quotes)

"true" "true" ""

I need 512 bytes with Xcode. The sizeof operator returned 32, 24 and 32 for IPCString, std::string and ShmVector, respectively. Therefore, it seems that I need 32+4 bytes for one "true" string and 32 for the empty string. Adding the ShmVector there, I need 36+36+32+32=136 bytes for the structures themselves. I calculated the size using sizeof(std::string), which is 24 here (an oversight), so that gives me (28+28+24)*5 = 400, which is not enough here.

My question here is how to determine how much memory I need for the segment? The data I want to write to the segment is known when the function is called and thus is its size.

EDIT: I changed the size to be:

size_t size = (sizeof(IPCString) + loop_val.size() + sizeof(IPCString) + should_intercept.size() + sizeof(IPCString) + post_data.size() + sizeof(ShmVector)) * 2 + 500;

So far, so good. I always have less than 500 bytes of free space after I'm finished writing to the segment. I've tried with various data sizes, ranging from less than 100 bytes to 3 megabytes. I've tried without multiplying by 2 but, in that case, when I try to insert large data chunks, the program crashes as 500 additional bytes does not provide sufficient leeway.

If there is no answer in a couple of days, I'll post this edit as an answer.

Community
  • 1
  • 1
nm_tp
  • 463
  • 3
  • 15
  • Looks like you're not considering alignment in your calculation. – okovko Jan 18 '19 at 16:32
  • I supposed this is in part due to this. Any hints on how to take this into consideration? – nm_tp Jan 18 '19 at 16:34
  • The easiest way to do this is to `sizeof` a struct you declare that has members for all the things you're sizing up. The compiler will add padding for alignment. – okovko Jan 18 '19 at 16:37
  • I can try that but I mostly worry about the overhead Boost is adding besides the data structures that I insert, before I even insert anything. – nm_tp Jan 21 '19 at 07:37
  • There would be no overhead. sizeof evaluates at compile time. Although you need to understand the implementation of the boost module you’re using to really make sense of how to write this code. Or find a relevant example in their docs. It is really bizarre to see highly templated code that deals directly with shared memory. – okovko Jan 21 '19 at 17:32
  • Well, here is a hunch: try taking the sizes of the IPCStrings you’re actually pushing into the shared vector. You’re assuming they have the same size and byte usage as your parameters to write_to_memory. Let’s test that assumption. – okovko Jan 21 '19 at 17:42
  • Can you also edit the usage of your code into this: https://wandbox.org/permlink/2NKok8B9XudZlnkD – okovko Jan 21 '19 at 19:05
  • @okovko I've actually tried using `sizeof IPCString` and `sizeof ShmVector` yesterday but didn't get around to posting it. Look at my edit. Also, an assumption that IPCString and std::string take up the same amount of space is wrong, I've seen it for myself. I was not worried about the overhead I might add but if I'd add sufficient space to cover the overhead Boost adds. – nm_tp Jan 22 '19 at 09:12
  • `It is really bizarre to see highly templated code that deals directly with shared memory.` I'm having the same feelings. It's okay to insert and read custom data from boost without serializing but Boost, for some reason, does not do the allocation. It's like they gave up on trying to make this user friendly halfway through the implementation. – nm_tp Jan 22 '19 at 09:13
  • If you put a usage example into the wandbox link I gave you and give me the new link, then I can help you further and we can probably figure out the exact calculation that you need so you don't need to multiply by an arbitrary constant when allocating. – okovko Jan 22 '19 at 09:39
  • Also, since these are strings and not cstrings, shouldn't `IPCString shm_loop_val(charallocator);` be `IPCString shm_loop_val(stringallocator);` instead? The same would apply for `shm_should_intercept` and `shm_intercepted_data`. – okovko Jan 22 '19 at 09:43
  • You're right about the charallocator, but I took a more in depth look at your code and at the Boost module you're using. See my updated answer. – okovko Jan 22 '19 at 20:52

2 Answers2

0

You are making many assumptions about the equivalence of type sizes. The memory usage of a c string is not the same as the memory usage of a std::string which is not the same as the memory usage of an IPCString which is actually a vector, so you're not interested in size or length, but capacity.

Size your components from the horse's mouth: take the sizeof of the IPCString variables instead of the std::string variables, and also take shm_loop_val.capacity() + shm_should_intercept.capacity() + shm_intercepted_data.capacity() instead of sizing std::strings!

You should be closer with:

size_t size = (3 * sizeof(IPCString) + sizeof(ShmVector) + shm_loop_val.capacity() +
              shm_should_intercept.capacity() + shm_intercepted_data.capacity();

The Boost implementation might also word align your data, but I'm starting to realize that's very unlikely. This looks like it's serialized. We can look into that you're still having issues.

okovko
  • 1,851
  • 14
  • 27
  • I believe we are passing the allocator for the **elements** of the structure. In the case of `IPCString`, the elements are 'chars' and in the case of `ShmVector`, the elements are `IPCString`s. I got the idea from: https://stackoverflow.com/questions/12980716/create-a-shared-memory-vector-of-strings – nm_tp Jan 22 '19 at 12:43
  • @nm_tp I see, it stands for Interprocess CString. I thought IPC was the prefix and these were string objects. It's good to use snake case after an acronym, so IP_CString. – okovko Jan 22 '19 at 20:32
  • It would actually be `IPC_String`. `IPC` as in interprocess communication. I shall post an answer that got me through without any bad_alloc exceptions these days. – nm_tp Jan 23 '19 at 09:31
  • @nm_tp Glad you figured it out on your own. – okovko Jan 23 '19 at 09:36
  • Also, about the `capacity()` method, I can't really use it. I need `size` to open and allocate the memory for the segment but I need the instances of `IPCString` for determining the size. I've had better luck with `sizeof IPCString + loop_val.size()`, since `size()` returns the length of the string _in bytes_. – nm_tp Jan 23 '19 at 09:42
  • @nm_tp You're storing vectors, not strings. Look at the implementation of IPCString. The reason you need to double your storage is probably because the vector is implemented by maintaining ~2 times the capacity that is actually being used at a given time. And the reason you don't use all the space is because the vector isn't going to be half full most of the time. – okovko Jan 23 '19 at 22:43
  • ` I need size to open and allocate the memory for the segment but I need the instances of IPCString for determining the size.` You don't see the circular dependency here? Please provide some proof for your statements. – nm_tp Jan 24 '19 at 09:26
0

After some research and testing, I came up with the solution below. The key point here is to use boost::move (or, perhaps std::move, don't know if there will be difference here). Because we're using boost::move, there will be no copies of the IPCString objects and, therefore, no need to multiply by 2. Remember, because of how we defined IPCString, it will be allocated inside the shared memory segment.

Added overhead is necessary (perhaps because of alignment or some other overhead that's added or both) but there is always some space left. Because I could happen to send several megabytes, the overhead of 500 bytes is pretty small.

I am using std::string's size() method because it returns the size of the string in bytes.

The code follows:

shared_memory.h:

#pragma once

#include <boost/interprocess/managed_shared_memory.hpp>
#include <boost/interprocess/containers/vector.hpp>
#include <boost/interprocess/containers/string.hpp>

#include <boost/interprocess/allocators/allocator.hpp>
#include <boost/interprocess/sync/named_mutex.hpp>

#include <string>
#include <vector>
#include <exception>

namespace apl_shared_memory
{
  typedef boost::interprocess::allocator<char, boost::interprocess::managed_shared_memory::segment_manager> CharAllocator;
  typedef boost::interprocess::basic_string<char, std::char_traits<char>, CharAllocator> IPCString;
  typedef boost::interprocess::allocator<IPCString, boost::interprocess::managed_shared_memory::segment_manager> StringAllocator;
  typedef boost::interprocess::vector<IPCString, StringAllocator> ShmVector;

  bool write_to_memory(std::string wsuid, std::string loop_val, std::string should_intercept, std::string post_data) ;


  const std::string shm_prefix = "shm_";
  const std::string mutex_prefix = "mtx_";
}

shared_memory.cpp:

#include "shared_memory.h"

#include "logger.h"



namespace apl_shared_memory

{
    bool write_to_memory(std::string wsuid, std::string loop_val, std::string should_intercept, std::string post_data)
    {
        bool ret_val;
        std::string shm_name = shm_prefix + wsuid;
        std::string mtx_name = mutex_prefix + wsuid;

        boost::interprocess::named_mutex named_mtx{ boost::interprocess::open_or_create, mtx_name.c_str() };

        // Add the size of all the structures we're putting in the shared memory region and add some for the overhead.
        size_t size = (3*sizeof(IPCString) + loop_val.size() + should_intercept.size() + post_data.size() + sizeof(ShmVector)) + 500;

        try
        {
            named_mtx.lock();
            boost::interprocess::shared_memory_object::remove(shm_name.c_str());
            boost::interprocess::managed_shared_memory segment(boost::interprocess::create_only, shm_name.c_str(), size);

            CharAllocator     charallocator(segment.get_segment_manager());
            StringAllocator   stringallocator(segment.get_segment_manager());

            IPCString shm_loop_val(charallocator);
            IPCString shm_should_intercept(charallocator);
            IPCString shm_intercepted_data(charallocator);

            shm_loop_val = loop_val.c_str();
            shm_should_intercept = should_intercept.c_str();
            shm_intercepted_data = post_data.c_str();

            segment.destroy<ShmVector>("ShmVector");
            ShmVector *shmVector = segment.construct<ShmVector>("ShmVector")(stringallocator);
            shmVector->clear();
            shmVector->reserve(3);

            // push_back will call a copy-constructor. But, if we send a rvalue reference (i.e. if we move it), there will be no copying.
            shmVector->push_back(boost::move(shm_loop_val));
            shmVector->push_back(boost::move(shm_should_intercept));
            shmVector->push_back(boost::move(shm_intercepted_data));

            ret_val = true;
        }

        catch (const std::exception& ex)

        {
            ret_val = false;
            boost::interprocess::shared_memory_object::remove(shm_name.c_str());
        }

        named_mtx.unlock();

        return ret_val;
    }
}

If anyone believes I did something wrong, please comment.

nm_tp
  • 463
  • 3
  • 15
  • The memory usage of an IPCString is not the size in bytes of the c string equivalent. – okovko Jan 23 '19 at 22:47
  • Care to elaborate? I see you're very invested in this but, so far, you've given me no usable solution and/or hints. You're just telling me that something is wrong but you're not providing any explanation whatsoever. – nm_tp Jan 24 '19 at 09:25
  • `typedef boost::interprocess::basic_string, CharAllocator> IPCString;` This line says elements of the basic_string are `char`s. `size()` method returns _the number of elements_ but `sizeof(char)` is 1 so, in this case, I can use `std::string`'s `size()` to determine the number of bytes I'll need, not including the overhead. I see no logical explanation why capacity would be twice the size of the buffer I just inserted into the `IPCString`. – nm_tp Jan 24 '19 at 09:34