CODEWITHSUNDEEP
c++dllstructdestructor
David
David

Reputation: 31

C++ Struct with pointers : constructor and destructor

I am having a problem with a simulation program that calls a DLL to perform an optimization task. After having studied this issue for a certain time, I think my problem lies in the destructor I use to free memory after the DLL has returned the desired information. The simulation program was developed on Borland C++ Builder v6 and the DLL was developed on MS Visual C++ 2005.

For the simulation program (P) and the DLL to exchange data, I created two structures InputCPLEX and OutputCPLEX and a function optimize that takes two arguments: one object of type InputCPLEX and one object of type OutputCPLEX. Both structures are declared in a header file structures.h which belongs to the P project and the DLL project.

Both InputCPLEX and OutputCPLEX structures have int and int* members, so basically the file structures.h looks like : 

//structures.h
struct InputCPLEX{
  public:
  int i;
  int* inputData;
}
struct OutputCPLEX{
  public:
  int j;
  int* outputData;
}

The idea is that along the simulation process (the execution of P), I periodically call the DLL to solve an optimization problem, so inputData is the array corresponding to the variables in my optimization problem and outputData is the array of optimal values for my variables. I know that it would have been easier to use the STL containers, such as vector<int>, however - correct me if I am wrong - it seems it is difficult to exchange STL objects between two different compilers.

Here is how things look in my main file (in P):

//main.h
InputCPLEX* input;
OutputCPLEX* output;
int* var;
int* sol;

//main.cpp    
[...] //lots of code
input = new InputCPLEX;
output = new OutputCPLEX;
int n = X; //where X is an integer 
var = new int[n]; 
[...] //some code to fill var
input->i = n; 
input->inputData = var; 
optimize(input,output); //calls the DLL
int m = output->j; 
sol = new int[n];
sol = output->outputData;
[...] //some code to use the optimized data
delete[] var;
delete[] sol;
delete input;
delete output;
[...] //lots of code

For more than one year I have been using this code without any constructor or destructor in the file structures.h, so no initialization of the structures members was performed. As you may have guessed, I am no expert in C++, in fact it's quite the opposite. I also want to underline that I did not code most of the simulation program, just some functions, this program was developed for more than 10 years by several developers, and the result is quite messy.

However, everything was working just fine until recently. I decided to provide more information to the DLL (for optimization purposes), and consequently the simulation program has been crashing systematically when running large simulations (involving large data sets). The extra information are pointers in both structures, my guess is that the program was leaking memory, so I tried to code a constructor and a destructor so that the memory allocated to the structures input and output could be properly managed. I tried the following code which I found searching up the internet : 

//structures.h
struct InputCPLEX{
  public:
  int i;
  int* inputData;
  int* inputData2; // extra info
  int* inputData3; // extra info
  InputCPLEX(): i(0), inputData(0), inputData2(0), inputData3(0) {}
  ~InputCPLEX(){ 
    if (inputData) delete inputData;
    if (inputData2) delete inputData2;
    if (inputData3) delete inputData3;
  }
}
struct OutputCPLEX{
  public:
  int j;
  int* outputData;
  int* outputData2;
  int* outputData3;
  OutputCPLEX(): j(0), outputData(0), outputData2(0), outputData3(0) {}
  ~OutputCPLEX(){ 
    if (outputData) delete outputData;
    if (outputData2) delete outputData2;
    if (outputData3) delete outputData3;
  }
}

But it does not seems to work: the program crashes even faster, after only a short amount of time. Can someone help me identify the issues in my code? I know that there may be other factors affecting the execution of my program, but if I remove both constructors and destructors in structures.h file, then the simulation program is still able to execute small simulations, involving small data sets.

Thank you very much for your assistance, David.

Upvotes: 3

Views: 8955

Answers (3)

AndersK
AndersK

Reputation: 36102

This looks odd:

int m = output->j; 
sol = new int[n];
sol = output->outputData;

as far as I understood it you return the size in m but allocate with n then you overwrite the array by setting the pointer (sol) to outputData I think you meant something like:

int m = output->j; 
sol = new int[m];
memcpy(sol,output->outputData,sizeof(int)*m);

Upvotes: 0

Rost
Rost

Reputation: 9089

I see several problems in your code:

1) Memory leak/double deletion:

sol = new int[n];
sol = output->outputData;

Here you override sol pointer right after initialization and data allocated by new int[n] is leaked. Also you double delete pointer in sol - second time in destructor of output. The same problem with var - you delete it twice, by explicit delete[] and in destructor of input.

Double deletion problem is raised after you have added destructors with delete, looks like before it was not a problem.

Also as @Riga mentioned you use new[] to allocate array, but delete instead of delete[] in destructors. This is not correct and this is Undefined Behavior. Despite this doesn't look like crash cause. In real world most compilers don't make difference implementing delete and delete[] for built-in and POD types. Serious problems can arise only when you delete array of objects with non-trivial destructors.

2) Where output->outputData is allocated? If in DLL it is another problem, as you usually cannot safely deallocate memory in your main program if it was allocated in DLL implemented with another compiler. The reason is different new/delete implementation and different heaps used by runtimes of main program and DLL.

You always shall allocate/deallocate memory on same side. Or use some common lower-level API - e.g. VirtualAlloc()/VirtualFree() or HeapAlloc()/HeapFree() with same heap handle.

Upvotes: 1

Riga
Riga

Reputation: 2149

You have to use consistent way of new - delete. If something was acquired by new[] you should delete it by delete[], if by new -> delete by delete. In your code you create input and output by new but delete via delete[].

BTW, you do not have to check a pointer for zero before deletion. delete handles zero pointers with no problems.

Upvotes: 1

Related Questions