7

Hey am trying to create a shared object between 2 processes.and trying to read and change the values from each one of them.This s my simple struct.

EDIT: I added a constructor to my struct.

struct shared{
    shared(){
        value = 10;
        name = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
    }
    int value;
    string name;
};

I tried both to call shmat() before and after calling fork() but nothing change it still give segmentation fault.

EDIT:And added a check after the shmat() to see if it failed.

#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <iostream>
#include <sys/shm.h>
#include <string.h>

using namespace std;

struct shared{
    shared(){
        value = 10;
        name = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
    }
    int value;
    string name;
};

int main(){
    int shm_id = shmget(IPC_PRIVATE,sizeof(shared),0);

    if(shm_id == -1){
        cout<<"shmget() failed "<<endl;
        return -1;
    }
    pid_t pid = fork();
    if(pid == -1){
        cout<<"fork() failed "<<endl;
        return -2;
    }

    shared* sharedPtr = (shared*)shmat(shm_id,0,0);


    if(sharedPtr == 0){
        cout<<"shmat() failed "<<endl;
    }

    cout<<"Setting up the object: "<<endl;
    sharedPtr->value = 5;
    sharedPtr->name = "aaaaaa: ";

    if(pid == 0){       //Child process

        cout<<"Child process: "<<endl;
        sharedPtr->value = 10;
        sharedPtr->name  = "bbbbbbb: ";
        cout<<"Changed the object "<<endl;
        return 0;
    }
    if(pid != 0){ //Parent process
        sleep(1);
        cout<<"Parent process: "<<endl;
        cout<< sharedPtr->name << sharedPtr->value<<endl;

    }

    return 0;
}

But I still get a segmentation fault.

  • You are not checking your shmat cal for errors. – n. m. could be an AI Jan 08 '18 at 07:25
  • 1
    and this is not C. – Sourav Ghosh Jan 08 '18 at 07:26
  • Well, in fairness, *some* calls are being checked. However, I wonder at the wisdom of (for example) checking `shm_id` *after* you've tried to attach to it :-) Ditto for `sharedPtr`, you set the fields of the struct *before* you check it for `nullptr`. – paxdiablo Jan 08 '18 at 07:28
  • 3
    Perhaps more importantly, you have an uninitialised `std::string` in shared memory. This is guaranteed to break. And you access it from different process without any synchronisation. This cannot possibly work either. The entire approach is deeply flawed. – n. m. could be an AI Jan 08 '18 at 07:30
  • 5
    Even if the `string` was initialised, you would have problems. While the string object itself is in shared memory, it uses memory outside its own object (for storage of the string characters). They won't be inside the shared memory. You cannot in general put a non-POD object in shared memory and expect it to work. – davmac Jan 08 '18 at 12:15
  • 1
    @davmac It's not about the POD though (i.e. trivially copyable). You *can* have an object with complex behavior. You *cannot* have an object with pointers to outside memory (in case you make sure to map the segment to the same address in all processes, which may fail if the virtual address range is already used), or even any pointer at all (if you, as usual, call `shmat` with a null pointer). A possible solution is to use an allocator based on the shared memory segment, and to store offsets instead of pointers. But `string` or the standard containers don't support that, so – a lot of work. – Arne Vogel Jan 08 '18 at 12:37
  • 1
    @ArneVogel you can't even have an object with pointers to _inside_ memory in the general case since they may be mapped at different addresses (not a problem if you share the memory by forking as in OP's example). I said "in general you cannot expect it to work" meaning that you would need to know specifics of internals before you could know whether it would work. Perhaps not the best wording, sure. (I replied without reading properly, it seems. You do mention the former point). – davmac Jan 08 '18 at 13:16

3 Answers3

4

The check for validity of the pointer (if(sharedPtr == 0){) occurs after the pointer has already been dereferenced in sharedPtr->value. So you could be getting this behaviour because you are dereferencing a null pointer. Put this check before you use the pointer.

Furthermore, you can't use shared memory like that. You are getting a statically sized shared memory pointer that represents memory equivalent to an uninitialized string and int, think about this. You essentially have this:

int : size -> 4 bytes
empty string : size -> 32 bytes

(From here, system dependent). Then when you do this (shared*)shmat(shm_id,0,0) and this sharedPtr->name = "aaaaaa: ", the string you have created in this struct never had its constructor called and therefore was not created correctly. When you do the operator= you are attempting to use a function that doesn't exist anywhere because the object you are calling it on wasn't constructed correctly. If you are using raw shared memory, you will need to be very careful about how you use it and how much size you will take up. You could also consider using a memory controller to help you, like boost::interprocess.

Fantastic Mr Fox
  • 32,495
  • 27
  • 95
  • 175
3

You have a crash because of incorrect shared memory key.

But actually you have a list problems of various severity:

1) IPC_PRIVATE is not suitable. You need to use ftok() like this:

key_t key;
if ((key = ftok("/path/to/existing/file", 'R')) == -1) {
  cerr << "ftok() failed: " << strerror(errno) << "\n";
  return -1;
}

2) This check should be performed right after shmget() call.

if(shm_id == -1){
    cout<<"shmget() failed "<<endl;
    return -1;
}

3) Use strerror(errno) to get a failure description as string

4) Here is an example on your topic https://stackoverflow.com/a/22557543/9187525

5) shmat() returns -1 on failure so your check is not correct.

6) std::string generally is not suitable because it use heap which is not seen by other process. You will get a crash depending on your C++ standard version and length of a string. See small string optimization (SSO) for details.

7) You need to call placement new after creating a shared memory segment to call a default ctor.

8) shmget() is obsolete and you probably want to use mmap() for shared memory, or even better corresponding boost library if you use C++.

Paul Coccoli
  • 546
  • 1
  • 4
  • 16
Eugene Kosov
  • 993
  • 7
  • 15
0

You need to set the permissions for the newly created segment in the call to shmget:

// read/write permissions ---------------------vvvv
int shm_id = shmget(IPC_PRIVATE,sizeof(shared),0666);

Then, you need to actually allocate the object in the shred memory segment.

Once you have already obtained the pointer, you need to create the object there using placement new:

new (sharedPtr) shared{}; // allocate the object in the shared memory segment

Working example.


Note that the string doens't get updated in the parent process after changing it in its child, but I think that has to do with the string implementation itself, not the shared memory management. (Correct me if I'm wrong)

amc176
  • 1,514
  • 8
  • 19