3

I'm trying to share an array of structs through shared named memory using the WINAPI. I'm able to create and manage the shared memory but when trying to share an array of structs the size of the array is always 0 upon reading.

Below is test code i have written which should write/read an array of 10 entries, but even this is failing. My goal is however to write/read a dynamic array of structs containing 2 dynamic arrays and the info they already contain at the moment.

I'm aware i shouldn't share pointers between processes as they could point to a random value. Therefor i'm allocating memory for the arrays using new.

This is what i have so far:

Shared in both processes:

#define MEMSIZE 90024 

typedef struct {
    int id;
    int type;
    int count;
} Entry;

Process 1:

extern HANDLE hMapObject;
extern void* vMapData;

std::vector<Entry> entries;//collection of entries

BOOL DumpEntries(TCHAR* memName) {//Returns true, writing 10 entries
    int size = min(10, entries.size());

    Entry* eArray = new Entry[size];
    for (int i = 0; i < size; i++) {
        eArray[i] = entries.at(i);
    }

    ::hMapObject = CreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0, MEMSIZE, memName);
    if (::hMapObject == NULL) {
        return FALSE;
    }

    ::vMapData = MapViewOfFile(::hMapObject, FILE_MAP_ALL_ACCESS, 0, 0, MEMSIZE);
    if (::vMapData == NULL) {
        CloseHandle(::hMapObject);
        return FALSE;
    }

    CopyMemory(::vMapData, eArray, (size * sizeof(Entry)));
    UnmapViewOfFile(::vMapData);
    //delete[] eArray;
    return TRUE;
}

Process 2:

BOOL ReadEntries(TCHAR* memName, Entry* entries) {//Returns true reading 0 entries
    HANDLE hMapFile = OpenFileMapping(FILE_MAP_ALL_ACCESS, FALSE, memName);
    if (hMapFile == NULL) {
        return FALSE;
    }

    Entry* tmpEntries = (Entry*)(MapViewOfFile(hMapFile, FILE_MAP_ALL_ACCESS, 0, 0, 10 * sizeof(Entry)));
    if (tmpEntries == NULL) {
        CloseHandle(hMapFile);
        return FALSE;
    }

    entries = new Entry[10];

    for (int i = 0; i < 10; i++) {
        entries[i] = tmpEntries[i];
    }

    UnmapViewOfFile(tmpEntries);
    CloseHandle(hMapFile);
    return TRUE;
}

Writing the 10 entries seems to be working but when trying to read the memory it returns successfully and the size of the array is 0, like so:

Entry* entries = NULL;
if (ReadEntries(TEXT("Global\Entries"), entries)) {
        int size = _ARRAYSIZE(entries);
        out = "Succesfully read: " + to_string(size);// Is always 0
}

So my question is, what am I doing wrong? I'm sharing the same struct between 2 processes, i'm allocating new memory for the entries to be written to and copying the memory with a size of 10 * sizeof(Entry);. When trying to read I also try to read 10 * sizeof(Entry); bytes and cast the data to a Entry*. Is there something I'm missing? All help is welcome.

MircoProgram
  • 295
  • 1
  • 20

3 Answers3

2

Based on cursory examination, this code appears to attempt to map structures containing std::strings into shared memory, to be used by another process.

Unfortunately, this adventure is doomed, before it even gets started. Even if you get the array length to pass along correctly, I expect the other process to crash immediately, as soon as it even smells the std::string that the other process attempted to map into shared memory segments.

std::strings are non-trivial classes. A std::string maintains internal pointers to a buffer where the actual string data is kept; with the buffer getting allocated on the heap.

You do understand that sizeof(std::string) doesn't change, whether the string contains five characters, or the entire contents of "War And Peace", right? Stop and think for a moment, how that's possible, in just a few bytes that it takes to store a std::string?

Once you think about it for a moment, it should become crystal clear why mapping one process's std::strings into a shared memory segment, and then attempting to grab them by another process, is not going to work.

The only thing that can be practically mapped to/from shared memory is plain old data; although you could get away with aggregates, in some cases, too.

Community
  • 1
  • 1
Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • Hey, thanks for your comment. I'm not really trying to share `std::string` because I'm using `TCHAR*` for that purpose, i just added it to OP as example. My biggest concern about shared named memory is to actually be able to share a `std::vector` or an array containing `POINT`. Or better, being able to send a collection of points (x, y) from one processs to another. A 1D array of ints is also good. – MircoProgram Jun 18 '16 at 08:55
  • Updated the question – MircoProgram Jun 21 '16 at 07:09
2

I'm afraid the problem only lies in the _ARRAYSIZE macro. I could not really find it in MSDN, but I found references for _countof or ARRAYSIZE in other pages. All are defined as sizeof(array)/sizeof(array[0]). The problem is that it only make sense for true arrays defined as Entry entries[10], but not for a pointer to such an array. Technically when you declare:

Entry* entries;

sizeof(entries) is sizeof(Entry *) that is the size of a pointer. It is smaller than the size of the struct so the result of the integer division is... 0!

Anyway, there are other problems in current code. The correct way to exchange a variable size array through shared memory is to use an ancillary structure containing a size and the array itself declared as incomplete:

struct EntryArray {
    size_t size;
    Entry entries[];
};

You could dump it that way:

BOOL DumpEntries(TCHAR* memName) {//Returns true, writing 10 entries
    int size = min(10, entries.size());

    EntryArray* eArray = (EntryArray *) malloc(sizeof(EntryArray) + size * sizeof(Entry));
    for (int i = 0; i < size; i++) {
        eArray->entries[i] = entries.at(i);
    }
    eArray->size = size;

    ::hMapObject = CreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0, MEMSIZE, memName);
    if (::hMapObject == NULL) {
        return FALSE;
    }

    ::vMapData = MapViewOfFile(::hMapObject, FILE_MAP_ALL_ACCESS, 0, 0, MEMSIZE);
    if (::vMapData == NULL) {
        CloseHandle(::hMapObject);
        return FALSE;
    }

    CopyMemory(::vMapData, eArray, (sizeof(EntryArray) + size * sizeof(Entry)));
    UnmapViewOfFile(::vMapData);
    free(eArray);
    return TRUE;
}

You can note that as the last member of the struct is an incomplete array, it is allocated 0 size, so you must allocate the size of the struct + the size of the array.

You can then read it from memory that way:

size_t ReadEntries(TCHAR* memName, Entry*& entries) {//Returns the number of entries or -1 if error
    HANDLE hMapFile = OpenFileMapping(FILE_MAP_ALL_ACCESS, FALSE, memName);
    if (hMapFile == NULL) {
        return -1;
    }

    EntryArray* eArray = (EntryArray*)(MapViewOfFile(hMapFile, FILE_MAP_ALL_ACCESS, 0, 0, 10 * sizeof(Entry)));
    if (eArray == NULL) {
        CloseHandle(hMapFile);
        return -1;
    }

    entries = new Entry[10]; // or even entries = new Entry[eArray->size];

    for (int i = 0; i < 10; i++) { // same: i<eArray->size ...
        entries[i] = eArray->entries[i];
    }

    UnmapViewOfFile(eArray);
    CloseHandle(hMapFile);
    return eArray.size;
}

But here again you should note some differences. As the number of entries is lost when eArray vanishes, it is passed as the return value from the function. And and you want to modify the pointer passed as second parameter, you must pass it by reference (if you pass it by value, you will only change a local copy and still have NULL in original variable after function returns).

There are still some possible improvement in your code, because the vector entries is global when it could be passed as a parameter to DumpEntries, and hMapObject is also global when it could be returned by the function. And in DumpObject you could avoid a copy by building directly the EntryArray in shared memory:

HANDLE DumpEntries(TCHAR* memName, const std::vector<Entry>& entries) {
    //Returns HANDLE to mapped file (or NULL), writing 10 entries
    int size = min(10, entries.size());

    HANDLE hMapObject = CreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0, MEMSIZE, memName);
    if (hMapObject == NULL) {
        return NULL;
    }

    void * vMapData = MapViewOfFile(hMapObject, FILE_MAP_ALL_ACCESS, 0, 0, MEMSIZE);
    if (vMapData == NULL) {
        CloseHandle(hMapObject);
        return NULL;
    }

    EntryArray* eArray = (EntryArray*) vMapData;
    for (int i = 0; i < size; i++) {
        eArray->entries[i] = entries.at(i);
    }
    eArray->size = size;

    UnmapViewOfFile(vMapData);
    return hMapObject;
}

And last but not least, the backslash \ is a special quoting character in a string litteral, and it must quote itself. So you should write .TEXT("Global\\Entries")

Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
  • Thank you for your help, how should dynamic arrays within the `Entry` struct be handled? I will test this out soon. – MircoProgram Jun 21 '16 at 14:24
1

I did it some changes to your code:

PROCESS 1:

BOOL DumpEntries(TCHAR* memName)
{
     int size = entries.size() * sizeof(Entry) + sizeof(DWORD);

     ::hMapObject = CreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0, size, memName);
     if (::hMapObject == NULL) {
          return FALSE;
     }

     ::vMapData = MapViewOfFile(::hMapObject, FILE_MAP_ALL_ACCESS, 0, 0, size);
     if (::vMapData == NULL) {
          CloseHandle(::hMapObject);
          return FALSE;
     }

     (*(DWORD*)::vMapData) = entries.size();
     Entry* eArray = (Entry*)(((DWORD*)::vMapData) + 1);
     for(int i = entries.size() - 1; i >= 0; i--) eArray[i] = entries.at(i);

     UnmapViewOfFile(::vMapData);
     return TRUE;
}

PROCESS 2:

BOOL ReadEntries(TCHAR* memName, Entry** entries, DWORD &number_of_entries) {
     HANDLE hMapFile = OpenFileMapping(FILE_MAP_ALL_ACCESS, FALSE, memName);
     if (hMapFile == NULL) {
          return FALSE;
     }

     DWORD *num_entries = (DWORD*)MapViewOfFile(hMapFile, FILE_MAP_ALL_ACCESS, 0, 0, 0);
     if (num_entries == NULL) {
          CloseHandle(hMapFile);
          return FALSE;
     }
     number_of_entries = *num_entries;

     if(number_of_entries == 0)
     {
         // special case: when no entries was found in buffer
         *entries = NULL;
         return true;
     }

     Entry* tmpEntries = (Entry*)(num_entries + 1);

     *entries = new Entry[*num_entries];

     for (UINT i = 0; i < *num_entries; i++) {
          (*entries)[i] = tmpEntries[i];
     }

     UnmapViewOfFile(num_entries);
     CloseHandle(hMapFile);

     return TRUE;
}

PROCESS 2 (usage example):

void main()
{
    Entry* entries;
    DWORD number_of_entries;

    if(ReadEntries(TEXT("Global\\Entries", &entries, number_of_entries) && number_of_entries > 0)
    {
        // do something
    }
    delete entries;
}

CHANGES:

  • I am not using a static size (MEMSIZE) when i map memory, i am calculating exactly memory requiered
  • I put a "header" to memory mapped, a DWORD for send to process 2 number of entries in buffer
  • your ReadEntries definition is wrong, i fix it changing Entry* to Entry**

NOTES:

  • you need to close ::hMapObject handle in process 1 before process 2 calls ReadEntries
  • you need to delete entries memory returned for ReadEntries in process 2, before you use it
  • this code works only under same windows user, if you want to communicate a services with user process (for example), you need to handle SECURITY_ATTRIBUTES member in CreateFileMapping procedure
Ing. Gerardo Sánchez
  • 1,607
  • 15
  • 14
  • This looks promising, will try later and let you know. – MircoProgram Jun 25 '16 at 09:35
  • When creating a `TEXT("Local\\Entries")` memory I should be able to read the memory in a different process on the same computer right? When trying to create Local memory upon reading it can't find the memory and prints 'File not found'. – MircoProgram Jun 25 '16 at 13:46
  • **TEXT("Global\\Entries")** works with all process, **TEXT("Local\\Entries")** works with process in same username space. This is better explained here: https://msdn.microsoft.com/en-us/library/windows/desktop/aa382954(v=vs.85).aspx – Ing. Gerardo Sánchez Jun 25 '16 at 23:25
  • So far going good, just one tiny more problem. The reading almost fully works now. I'm able to read the size of the array but the code crashes and gives a null pointer here `*entries = new Entry[*num_entries];` I call the method like this however `Entry **entries = NULL; DWORD size; if (ReadEntries(TEXT("Global\\Entries"), entries, size)) { //Do stuff }` – MircoProgram Jun 26 '16 at 13:56
  • I added a usage example for proccess 2. Be sure you call DumpEntries in process 1 with values (entries.size() > 0) – Ing. Gerardo Sánchez Jun 26 '16 at 14:15
  • Thank you for your great help, the memory works now and assigned you the bounty. There's some new things you have shown me I didn't know about C++. However it would be great if you could lead me on how to add a dynamic array to the `Entry` struct, for example: `int values[];` – MircoProgram Jun 26 '16 at 15:57
  • That is another (and more simple) question, i think. Please ask it in other question – Ing. Gerardo Sánchez Jun 27 '16 at 11:50
  • I created a new question: https://stackoverflow.com/questions/38128514/c-winapi-shared-memory-dynamic-arrays – MircoProgram Jun 30 '16 at 16:17