C++

Forum

 
Problems with delete[] inside of ~destructor

margareto (11)
Hi,
I am having problems while I am trying do delete[] an array from a struct. Actually, I am defining the struct with a destructor, in the following way:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
 
struct EquitySwapParameters{
	double	Redemption_Rate;
	double Accrued_Interest_Floating;
	double Accrued_Factor_Basket;
	int Accrued_Index_Basket;

	double*	Discount_Factors_Maturity;	//!< Dimension R		
	long	Maturity_Date;
	long	Valuation_Date;
	int		Number_of_Observation_Dates;//!< Dimension M		
	long*	Observation_Dates;			//!< Dimension M
	int		Number_of_Equities;			//!< Dimension N
	double*	Weights;					//!< Dimension N		
	double*	Equity_Prices;				//!< Dimension N x M.
	int		Number_of_Payment_Dates;	//!< Dimension Q
	long*	Payment_Dates;				//!< Dimension Q
	double*	Period_Lengths;				//!< Dimension Q
	double*	Discount_Factors;			//!< Dimension R x Q
	double*	Caps;						//!< Dimension Q
	double*	Floors;						//!< Dimension Q
	int		Number_of_Extra_Variables;	//!< Dimension T
	double*	Extra_Variables;			//!< Dimension T
	int		PayOff_Number;
	int		Rho_Start_Index;			
	int		Is_Equity_Swap;				
	int		Number_of_Floating_Payment_Dates;//!< Dimension U
	long*	Floating_Payment_Dates;		//!< Dimension U
	double*	Floating_Payments;			//!< Dimension R x U
	double*	Floating_Discount_Factors;	//!< Dimension R x U
	int		Calculate_Expected_Coupon_Rates;

	~EquitySwapParameters()
	{
		if (Discount_Factors_Maturity){
			delete[] Discount_Factors_Maturity;
			Discount_Factors_Maturity = NULL;
		}
		if (Observation_Dates){
			delete[] Observation_Dates;
			Observation_Dates = NULL;
		}
		if (Weights){
			delete[] Weights;
			Weights = NULL;
		}
		if (Payment_Dates){
			delete[] Payment_Dates;
			Payment_Dates = NULL;
		}
		if (Period_Lengths){
			delete[] Period_Lengths;
			Period_Lengths = NULL;
		}
		if (Discount_Factors){
			delete[] Discount_Factors;
			Period_Lengths = NULL;
		}
		if (Caps){
			delete[] Caps;
			Caps = NULL;
		}
		if (Floors){
			delete[] Floors;
			Floors = NULL;
		}
		if (Extra_Variables){
			delete[] Extra_Variables;
			Extra_Variables = NULL;
		}
		if (Floating_Payment_Dates){
			delete[] Floating_Payment_Dates;
			Floating_Payment_Dates = NULL;
		}
		if (Floating_Payments){
			delete[] Floating_Payments;
			Floating_Payments = NULL;
		}
		if (Floating_Discount_Factors){
			delete[] Floating_Discount_Factors;
			Floating_Discount_Factors = NULL;
		}
	}
};


So, when I am debugging, it suddenly stops in:
1
2
3
4
 
		if (Extra_Variables){
			delete[] Extra_Variables;
			Extra_Variables = NULL;
		}


Concretely in: delete[] Extra_Variables;

It is some kind of exception, but actually I have no explanation and any exception is showed by the debug. If I comment those lines, I see how it occurs in the following statement of delete[]. I have no idea where I could be wrong. I make the respective news for the pointers in other function, and I don't want take care of deleting, that's the reason why I made such destructor. I call it like that:
 
 
delete info; //info's type "EquitySwapParameters *"


My code is a legacy, so I can not change the struct (no troubles for a destructor, of course).

Could you tell me where is my problem? Why doesn't it crash in the previous delete[] or in the next one? Is that a bad practice?

Thanks in advance!

R0mai (730)
delete implicitly checks for null pointer, so no need for all those ifs. Also, since those pointers, which you are assigning NULL to after their deletion, are going to get destructed right at the end of the destructor, so it really makes no sense to assign anything to them.
So your destructor boils down to this:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
~EquitySwapParameters() {

    delete[] Discount_Factors_Maturity;
    delete[] Observation_Dates;
    delete[] Weights;
    delete[] Payment_Dates;
    delete[] Period_Lengths;
    delete[] Discount_Factors;
    delete[] Caps;
    delete[] Floors;
    delete[] Extra_Variables;
    delete[] Floating_Payment_Dates;
    delete[] Floating_Payments;
    delete[] Floating_Discount_Factors;
}


Now, this probably doesn't solve your problem. What I think the problem is, that you allocate some of these with new, not new[], or you doesn't allocate them at all. Please post the function(s) which initializes these pointers.

Last edited on
margareto (11)
Thanks! I consider it, but as you tell it doesn't solve my problem.
This is the code which initializes these pointers:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
 
EquitySwapParameters* IOData::ReadInfoFromFile(ifstream& inputFile, EquitySwapParameters *Info)
{
	string line;

	inputFile >> Info->Accrued_Factor_Basket;
	inputFile >> Info->Accrued_Index_Basket;
	inputFile >> Info->Accrued_Interest_Floating;
	inputFile >> Info->Is_Equity_Swap;
	inputFile >> Info->Maturity_Date;
	inputFile >> Info->Number_of_Equities;
	inputFile >> Info->Number_of_Extra_Variables;
	inputFile >> Info->Number_of_Floating_Payment_Dates;
	inputFile >> Info->Number_of_Observation_Dates;
	inputFile >> Info->Number_of_Payment_Dates;
	inputFile >> Info->PayOff_Number;
	inputFile >> Info->Redemption_Rate;
	inputFile >> Info->Rho_Start_Index;
	inputFile >> Info->Valuation_Date;	
	inputFile >> Info->Calculate_Expected_Coupon_Rates;
	if (!inputFile.good()) return NULL;

	//Reservamos espacio para los arrays en tiempo de ejecución.
	Info->Caps = new double[Info->Number_of_Payment_Dates];
	Info->Discount_Factors = new double[(Info->Rho_Start_Index+1)*Info->Number_of_Payment_Dates];
	Info->Discount_Factors_Maturity = new double[Info->Rho_Start_Index+1];
	Info->Equity_Prices = new double[Info->Number_of_Observation_Dates];
	Info->Extra_Variables = new double[Info->Number_of_Extra_Variables];
	Info->Floating_Discount_Factors = new double[(Info->Rho_Start_Index+1)*Info->Number_of_Floating_Payment_Dates];
	Info->Floating_Payment_Dates = new long[Info->Number_of_Floating_Payment_Dates];
	Info->Floating_Payments = new double[(Info->Rho_Start_Index+1)* Info->Number_of_Floating_Payment_Dates];
	Info->Floors = new double[Info->Number_of_Payment_Dates];
	Info->Observation_Dates = new long[Info->Number_of_Observation_Dates];
	Info->Payment_Dates = new long[Info->Number_of_Payment_Dates];
	Info->Period_Lengths = new double[Info->Number_of_Payment_Dates];
	Info->Weights = new double[Info->Number_of_Equities];

	for (int i=0; i<Info->Number_of_Payment_Dates; i++)
	{		
		inputFile >> (Info->Caps[i]);
	}
	//Recorremos el array Discount_Factors primero por filas y luego por columnas
	//Ojo, "<=", ya que Rho_Start es un indice y no un número de items
	for (int i=0; i<=Info->Rho_Start_Index; i++)
	{		
		for (int j=0; j<Info->Number_of_Payment_Dates; j++){
			inputFile >> Info->Discount_Factors[i*Info->Number_of_Payment_Dates+j];
		}
	}
	for (int i=0; i<=Info->Rho_Start_Index; i++)
	{	
		inputFile >> Info->Discount_Factors_Maturity[i];
	}
	for (int i=0; i<Info->Number_of_Equities; i++)
	{
		for(int j=0; j<Info->Number_of_Observation_Dates; j++)
		{
			inputFile >> Info->Equity_Prices[i*Info->Number_of_Observation_Dates + j];
		}
	}
	for(int i=0; i<Info->Number_of_Extra_Variables; i++)
	{			
		inputFile >> Info->Extra_Variables[i];
	}
	for (int i=0; i<=Info->Rho_Start_Index; i++)
	{
		for (int j=0; j<Info->Number_of_Floating_Payment_Dates; j++)
		{
			inputFile >> Info->Floating_Discount_Factors[i*Info->Number_of_Floating_Payment_Dates +j];
		}
	}
	for (int i=0; i<Info->Number_of_Floating_Payment_Dates; i++)
	{
		inputFile >> Info->Floating_Payment_Dates[i];
	}
	for (int i=0; i<=Info->Rho_Start_Index; i++)
	{
		for (int j=0; j<Info->Number_of_Floating_Payment_Dates; j++)
		{
			inputFile >> Info->Floating_Payments[i*Info->Number_of_Floating_Payment_Dates + j];
		}
	}
	for (int i=0; i<Info->Number_of_Payment_Dates; i++)
	{
		inputFile >> Info->Floors[i];
	}
	for (int i=0; i<Info->Number_of_Observation_Dates; i++)
	{
		inputFile >> Info->Observation_Dates[i];
	}
	for (int i=0; i<Info->Number_of_Payment_Dates; i++)
	{
		inputFile >> Info->Payment_Dates[i];
	}
	for (int i=0; i<Info->Number_of_Payment_Dates; i++)
	{	
		inputFile >> Info->Period_Lengths[i];
	}
	for (int i=0; i<Info->Number_of_Equities;i++)
	{
		inputFile >> Info->Weights[i];
	}
	inputFile >> line;
	return Info;
}


Then I call last function from this other code:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
 
void IOData::LoadTests(int payOffNumber)
{
	string fileName = "\\Test";
	TestCase *testCase = new TestCase();
	EquitySwapParameters *info=NULL;
	stringstream sstr;
	bool testSuccessfully = true;
	unsigned int nTestCases=0;

	sstr << payOffNumber;
	fileName.append(sstr.str());
	dataPathInfo = dataPathTest;
	dataPathInfo.append(fileName);
	inputFile.open(dataPathInfo.c_str(),std::ifstream::in);

	if (inputFile.is_open())
	{
		while (inputFile.good())
		{
			info = new EquitySwapParameters;
			//testCase = new TestCase();
			ReadInfoFromFile(inputFile,info);
			if (info!=NULL)
			{
				testCase->SetInfo(info);
				ReadOutputFromFile(inputFile, testCase);
				if (!testCase->Execute())
				{
					testSuccessfully = false;
					cout << "Test " << nTestCases << " failed." << endl;
				}
				nTestCases++;
			}
			delete info;
		}
		delete testCase;
		if (testSuccessfully)
			cout << "Test successfully!" << endl;
		cout << "Press any number to exit."<< endl;
		cin >> nTestCases;

	}
	inputFile.close();
}

Any comment about any other good practice is very welcome!
As I tell before, when I am debugging, it suddenly stops and doesn't tell anything about exceptions... is some kind of hanging...
Thanks in advance
imi (426)
Also, since those pointers, which you are assigning NULL to after their deletion, are going to get destructed right at the end of the destructor, so it really makes no sense to assign anything to them.

A lot of people recommend to always assign 0 after deletion of any pointer. It may seem redundant, but there are cases when it's not that obvious that a pointer is accessed - even from the destructor. And compiler remove the code for unnecessary assigns anyway. (But well.. there have been fought a lot of flame wars about this topic)

I see things like this alot:
1
2
 
#define SAFE_DELETE(x) delete x; x = 0;
#define SAFE_DELETE_ARRAY(x) delete [] x; x = 0;


It is some kind of exception, but actually I have no explanation and any exception is showed by the debug.

Would be helpful, if you post the exception text ;-). My crystal ball tip for today is: "Heap corruption".
If so, then you have a problem... ;).

General tip would be: Don't use arrays. Use std::vector or boost::array. It's just better. (Edit: I think you already figured that out, though.. poor margareto. "legacy" sucks.)

Ciao, Imi.
Last edited on
margareto (11)
Thanks by advices Imi!
Actually I began the code using vectors, but I had some troubles with memory allocation. I guess now that I wasn't using well news and deletes. But in that moment I though that it was inefficient and I used pointers! Anyway, I know that I am making something wrong, and I would like to know more about C++ and get familiarize with it, so I have to manage right the pointers.

I have no significant reports in the debugging. It fails just in that sentence of the destructor:
delete [] Extra_variables;

This is the track of Results:

'TestQMC.exe': se cargó 'C:\Documents and Settings\pc017\Mis documentos\PFC\QMC-PFC\Debug\TestQMC.exe', Símbolos cargados.
'TestQMC.exe': se cargó 'C:\WINDOWS\system32\ntdll.dll', No se cargaron símbolos.
'TestQMC.exe': se cargó 'C:\WINDOWS\system32\kernel32.dll', No se cargaron símbolos.
'TestQMC.exe': se cargó 'C:\WINDOWS\WinSxS\x86_Microsoft.VC80.DebugCRT_1fc8b3b9a1e18e3b_8.0.50727.42_x-ww_f75eb16c\msvcp80d.dll', Símbolos cargados.
'TestQMC.exe': se cargó 'C:\WINDOWS\WinSxS\x86_Microsoft.VC80.DebugCRT_1fc8b3b9a1e18e3b_8.0.50727.42_x-ww_f75eb16c\msvcr80d.dll', Símbolos cargados.
'TestQMC.exe': se cargó 'C:\WINDOWS\system32\msvcrt.dll', No se cargaron símbolos.
'TestQMC.exe': se cargó 'C:\WINDOWS\system32\user32.dll', No se cargaron símbolos.
'TestQMC.exe': se cargó 'C:\WINDOWS\system32\gdi32.dll', No se cargaron símbolos.
'TestQMC.exe': se cargó 'C:\Archivos de programa\Google\Google Desktop Search\GoogleDesktopNetwork3.dll', No se cargaron símbolos.
'TestQMC.exe': se cargó 'C:\WINDOWS\system32\advapi32.dll', No se cargaron símbolos.
'TestQMC.exe': se cargó 'C:\WINDOWS\system32\rpcrt4.dll', No se cargaron símbolos.
'TestQMC.exe': se cargó 'C:\WINDOWS\system32\secur32.dll', No se cargaron símbolos.
'TestQMC.exe': se cargó 'C:\WINDOWS\system32\ws2_32.dll', No se cargaron símbolos.
'TestQMC.exe': se cargó 'C:\WINDOWS\system32\ws2help.dll', No se cargaron símbolos.

And this is the track of the heap:
TestQMC.exe!EquitySwapParameters::~EquitySwapParameters() Línea 65 C++
TestQMC.exe!EquitySwapParameters::`scalar deleting destructor'() + 0x2b bytes C++
TestQMC.exe!IOData::LoadTests(int payOffNumber=25) Línea 315 + 0x2b bytes C++
TestQMC.exe!main(int argc=1, wchar_t * * argv=0x003b61e8) Línea 37 C++
TestQMC.exe!__tmainCRTStartup() Línea 586 + 0x19 bytes C
TestQMC.exe!mainCRTStartup() Línea 403 C
kernel32.dll!7c817077()

As I tell you, no exception text at all! Mystery...

Thanks!
imi (426)
As I tell you, no exception text at all! Mystery...


Then it's probably a heap corruption. What this means is, you are writing out of bound somewhere and writing over essential memory necessary to destroy the array later.

If you have the possibility to change it into std::vector, I would really really really recommend to do so. If not in this code, then at least in the code you sent in as sample if you apply for a job somewhere. ;-)

Actually I began the code using vectors, but I had some troubles with memory allocation. I guess now that I wasn't using well news and deletes.

I guess so too, as using std::vector usually means that you don't have any new and delete anymore. ;)

The big advantage that you get is, that many STL-implementations have a "safe debug mode" where it immediately detects out-of-bound access. Then you see the real spot where your problem is and not the mere symptom later somewhere completely else..

(If your STL-implementation doesn't have a debug-mode that does iterator checking, you probably won't be better of for this specific case here. But it still helps in memory management, e.g. as you don't need to write all those delete's anymore)



If you still want to go for C-arrays, check your loops and all places where you assign any data to the array and make extra sure, that the index stays inside the allowed range.

Ciao, Imi.
Last edited on
imi (426)
1
2
3
4
5
6
7
8
9
 
	Info->Equity_Prices = new double[Info->Number_of_Observation_Dates];
...
	for (int i=0; i<Info->Number_of_Equities; i++)
	{
		for(int j=0; j<Info->Number_of_Observation_Dates; j++)
		{
			inputFile >> Info->Equity_Prices[i*Info->Number_of_Observation_Dates + j];
		}
	}


Ciao, Imi.
PS: But promise to use std::vector next time, ok?
margareto (11)
Ok, I have solved it!! Thanks imi! It was what you said about writing in a wrong site.

Look at this: 
1
2
3
4
5
6
7
8
9
10
 
	Info->Equity_Prices = new double[Info->Number_of_Observation_Dates];
        ....
        ....
        for (int i=0; i<Info->Number_of_Equities; i++)
	{
		for(int j=0; j<Info->Number_of_Observation_Dates; j++)
		{
			inputFile >> Info->Equity_Prices[i*Info->Number_of_Observation_Dates + j];
		}
	}


That's the reason why it failed when it tried delete the next pointer.

Also other mistake. That is my new code:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
		
while (inputFile.good())
		{

			info = new EquitySwapParameters;
			if (ReadInfoFromFile(inputFile,info)!=NULL)
			{
				testCase->SetInfo(info);
				ReadOutputFromFile(inputFile, testCase);
				if (!testCase->Execute())
				{
					testSuccessfully = false;
					cout << "Test " << nTestCases << " failed." << endl;
				}
				nTestCases++;
				delete info;
			}
		}	
		//delete testCase;


I got another hanging up when I tried delete a pointer in which I didn't write. I observed that when you receive a pointer as a parameter in a function, if you assign such pointer to NULL, it doesn't care about it, and after the function it will have stored the same memory address. I guess I got mistake again thinking that pointers don't behave like other "variable" passed by value!

I had to comment also:
delete testCase;

Implicitly it calls to the destructor of EquitySwapParameters and it fails.

Finally, I won't delete those last two news (for Info and testCase), but I don't care since it is just in last iteration and now program works fine.

Imi: I have a question about the treatment of vectors about pointers:
If I make a new of a struct (like mine) and I put it inside a vector (considering that I did news for the pointers of my struct), should I make delete in any moment?

Thanks a lot!! I will consider your advice of writing with STL library if I apply for some job!! Jajaja! But you never now when you will find a legacy with some evil pointers... ;)

Grazie mille!

PS: Any comments welcome again!


imi (426)
Finally, I won't delete those last two news (for Info and testCase), but I don't care since it is just in last iteration and now program works fine.

Consider not to create them with new at all. There's no obvious reason in your code to create them on the heap, so just write TestCase testCase; and use it with testCase.SetInfo.... Then you don't need any delete and no memory leaks out.


Imi: I have a question about the treatment of vectors about pointers:
If I make a new of a struct (like mine) and I put it inside a vector (considering that I did news for the pointers of my struct), should I make delete in any moment?

if you put any pointers that have been created with new into vectors, then you have to delete them.

But you don't have to delete the vector itself (except you created the vector itself with new, but why would you do this?)

Maybe it becomes clear if you think of a std::vector<double> as a 1:1 replacement of a "new double[]" (it's not a 1:1, but roughly.. ;)

1
2
3
4
5
6
7
8
9
10
11
 
// creating
double* c_style = new double[n];
std::vector<double> cpp_style(n);

// assigning values
for (int i = 0; i < n; ++i) c_style[i] = 23.42;
for (int i = 0; i < n; ++i) cpp_style[i] = 23.42;

// deletion
delete [] c_style;
// nothing to do for cpp_style... use "cpp_style.clear()" to explicitly free the values.


After you mastered this way of using std::vector, look into the more advanced stuff you can do with vector's and how they are usually used in C++ programs (iterators, for_each etc).

The same way as you instinctively won't write double** a[100] (which creates an array of 100 different pointers to double), you shouldn't write std::vector<double*> (except, of course, you really want an array of 100 pointers instead of 100 double values).

Now if you thought this through for double, it isn't any different with structs. If you would write MyStruct* p = new MyStruct[100]; write instead: std::vector<MyStruct> v(100);. If you don't do any "new" for the structs in the first case (only new[] for the whole array), so you don't need any new in the second case.


Some different world will emerge when you start using inheritance and subclasses and have to store pointers created with new in arrays. Then it will get a bit hairy. But that's another story.

Ciao, Imi.
margareto (11)
Many thanks Imi!!

In truth I was fighting against the code for hours, but finally I (we) managed it! Next time with vectors! For me is a great jump to come from .NET, but C++ is convincing me...

Ciao,

Margareto
Topic archived. No new replies allowed.