Search code examples
c++new-operatorbinaryfilesdynamic-memory-allocationdelete-operator

Unexpected runtime error (segmentation fault)


I am currently optimizing a memory intensive application to occupy lesser memory. What I am trying to do in the following code is to allocate the file stream objects ifstream and ofstream dynamically in order to free them exactly after it's use is no longer needed. The code functions perfectly for allocation/de-allocation of ofstream but crashes at runtime due to a possible segmentation fault when the memory contents of ifstream is de-allocated. The following is a snippet of the original code:

#include <fstream>
using namespace std;

// Dummy class to emulate the issue at hand
class dummy {
    private:
    int randINT;
    static bool isSeeded; 
    public:
    dummy() { randINT=rand(); }
    int getVal() { return randINT; }
};
bool dummy::isSeeded=false;

int main(int argc, const char* argv[]) {
    // Binary file I/O starts here
    dummy * obj;
    ofstream * outputFile;
    ifstream * inputFile;
    outputFile=new ofstream("bFile.bin",ios::binary);
    if (!(*outputFile).fail()) {
        obj=new dummy;
        cout << "Value to be stored: " << (*obj).getVal() << "\n";
        (*outputFile).write((char *) obj, sizeof(*obj)); // Save object to file
        (*outputFile).close();
        delete obj;
        // don't assign NULL to obj; obj MUST retain the address of the previous object it pointed to
    } else {
        cout << "Error in opening bFile.bin for writing data.\n";
        exit(1);
    }
    delete outputFile; // This line throws no errors!
    inputFile=new ifstream("bFile.bin",ios::binary);
    if (!(*inputFile).fail()) {
        (*inputFile).read((char *) obj,sizeof(dummy)); // Read the object of type 'dummy' from the binary file and allocate the object at the address pointed by 'obj' i.e. the address of the previously de-allocated object of type 'dummy' 
        cout << "Stored Value: " << (*obj).getVal() << "\n";
        (*inputFile).close();
    } else {
        cout << "Error in opening bFile.bin for reading data.\n";
        exit(1);
    }
    delete inputFile; // Runtime error is thrown here for no reason!

    cout << "\n-----END OF PROGRAM-----\n";

}

The above code saves an object to a binary file when the user evokes save functions. As stated in the code above, the de-allocation of inputFile pointer throws a runtime error.

Please note that I am using clang(more specifically, clang++) to compile the project.

Thanks in advance.


Solution

  • std::basic_istream::read(char_type*address, streamsize count) reads up to count characters and places them at address. It does not create any objects at address, as you seem to believe. The address must point to a valid chunk of memory at least count*sizeof(char_type) in size. By passing the address of a deleted object, you violate that condition: your code is broken and segmentation fault is not unexpected.

    EDIT

    What you're doing is undefined behaviour, short UB. When you do that, nothing is guaranteed and any logic about what happens in which order invalid. The program is allowed to do anything, including immediately crashing, running for a while and then crashing, or "make demons fly out of your nose".

    In your case, I suspect that std::basic_istream::read() writing to unprotected memory causes some data to be overwritten, for example the address of another object, which later causes the segmentation fault. But this is pure speculation and not really worth pursuing.

    In your case no object is created. The binary file just contains some bytes. The read() copies them to the address provided, which was not reserved for that purpose. To avoid the UB, simply add

    obj = new dummy;
    

    before the read to create an object.

    If you want to re-use the memory from the previous object, you could use placement new (see points 9 and 10 of that link). For example

    char*buffer = nullptr;                  // pointer to potential memory buffer
    if(writing) {
      if(!buffer)
        buffer = new char[sizeof(dummy)];   // reserve memory buffer
      auto pobj = new(buffer) dummy(args);  // create object in buffer
      write(buffer,sizeof(dummy));          // write bytes of object
      pobj->~pobj();                        // destruct object, but don't free buffer
    }
    if(reading) {
      if(!buffer)
        buffer = new char[sizeof(dummy)];   // not required if writing earlier
      read(buffer,sizeof(dummy));           // read bytes into object
      auto pobj = reinterpret_case<dummy*>(buffer); // no guarantees here
      use(pobj);                            // do something with the object read
      pobj->~pobj();                        // destruct object
    }
    delete[] buffer;                        // free reserved memory
    

    Note that if the reading does not generate a valid object, the later usage of that object, i.e. the call to its destructor, may crash.

    However, all this micro-optimisation is pointless anyway (it's only worth doing if you can avoid many calls to new and delete). Don't waste your time with that.