Access violation reading location Error when deleting the positions of a vector

[sarvcr]

Hi! I'm writting a code with several classes. One of them is supposed to contain a vector or objects from another class. I've done this kind of things several times, but this time I'm having an error that says : "Access violation reading location"

I have no clue about what can it be. I checked the code several times and everthing looks to be okay. Objects are being created properly and the syntax is the same I've used before.

The method generating the problem is this one:

  Sala::~Sala(){
	for (int i = 0; i < tamano; i++)
		delete butaca[i]; //<--- It's this exact line the one that is generating the problem. Text shown: Exception thrown at 0x00259BED in ProyectoCineV2.exe: 0xC0000005: Access violation reading location 0xDDDDDDDD.
	delete[]butaca;
}

I would really appreciate if you could help me. Let me know if you need the entire classes to find the problem. Thanks!

[AbstractionAnon]

You need to show the declaration of butaca.
What's the value of i when this fails?

Lines 3 and 4 imply the following:

  std::vector<somepointer *>  butaca;

The error message implies that you have an uninitialized pointer.

Line 2: Why are you using tamano and not butaca.size() ?
Relying on your own counter is subject to errors.
You should rely on the vector to tell you how many members it has.
It could be that tamano is larger than butaca.size() and you're trying to access beyond the end of the vector.

[sarvcr]

Tbh, I don't know the exact value of i when it fails, it just shuts the program down a second after executing it.

I'm creating the objects at int main(){} and then fill the entire vector with dynamic objects, so as far as I understand, no pointer should be uninitialized, but I'm probably wrong.

I'm using tamano and not butaca.size() because the vector belongs to the same class where the tamano is, butaca is the vector.

I'm showing the .cpp for this class to be more specific:

#include "Sala.h"

Sala::Sala(int elTamano) {
	cantidad = 0;
	tamano = elTamano;

	butaca = new Asiento * [tamano];
	for (int i = 0; i < tamano; i++) {
		butaca[i] = NULL;
	}
}
Sala::~Sala(){
	for (int i = 0; i < tamano; i++)
		delete butaca[i];
	delete[]butaca;
}

void Sala::setNombre(string elNombre) {
	nombre = elNombre;
}

string Sala::getNombre() {
	return nombre;
}

bool Sala::agregarButaca(Asiento* elem){
	if (cantidad < tamano) {
		butaca[cantidad] = elem;
		cantidad++;
		return true;
	}
	else {
		return false;
	}
}

[Ganado]

Show main, or if it's too long, at least show the parts where you call agregarButaca.

[sarvcr]

#include "ContTiquetes.h" //Contains "Tiquetes.h", who contains "Funcion.h", who contains "Sala.h"

int main() {
	//Crear Salas y asignarles Asientos
	int ocultar = 0;
	Sala SalaUno(60);
	SalaUno.setNombre("Sala 1");
	Sala SalaDos(60);
	SalaDos.setNombre("Sala 2");

	Asiento* ptrAsiento = NULL;
	do {

		ptrAsiento = new Asiento("A1", 'V');
		SalaUno.agregarButaca(ptrAsiento);
		SalaDos.agregarButaca(ptrAsiento);
                ptrAsiento = new Asiento("A2", 'V');
		SalaUno.agregarButaca(ptrAsiento);
		SalaDos.agregarButaca(ptrAsiento);
//These are just two objects to don't make it way too long. Actual code has 60 objects created this exact way
//Already checked if it worked by adding the pointer to just one vector at the time, didn't work.
        } while (ocultar != 0);
	
	return 0;
}

The do{}while() cicle is there just to hide the creation of the objects, it will only happen once

[Ganado]

You are passing the same pointer into two objects. Thus, when the Sala objects are destroyed, they are both calling delete on the same pointer, and you're double deleting your pointers.

You, as the designer of the software, need to decide who owns the dynamic memory you allocate with 'new'. Why do both Sala objects need to own the same exact memory? If the same memory should truly be shared between the two objects, I suggest using std::shared_ptr.

[sarvcr]

I see, thanks!

I'm gonna try it right now. I thought that doing it like that created a diferent object for each vector, looks like I didn't understand well

[AbstractionAnon]

I said before: Please show the declaration for butaca. i.e. sala.h
Please also show the declaration of Asiento.

I don't see where you're using std::vector.
Please don't refer to something as a vector, if you're not using std::vector.

Sala.cpp
line 7 butaca appears to be a simple array.
line 5: tamano is how big you want the array to be. It is NOT how many elements of the array are used.
Lines 8-9: You initialize the elements to NULL.
Lines 38-29: cantidad is your counter of how many elements are initialized.
Lines 13-14: You're looping for the number of allocated elements (tamano), not the number of elements used (cantidad).

You're going to get an illegal address reference as soon as your loop variable is equal to cantidad.

[sarvcr]

Thanks, it was what Ganado said, I thought that while doing what I made at main(), I was creating identical object, but looks like I was creating only one.

The illegal address was there because when the first vector deleted the class, the second one was trying to delete a non-existent object.

[AbstractionAnon]

While it is not illegal to release a null pointer, it's considered poor practice to do so.
Your loop limit variable at sala.cpp line 13 is wrong variable.

As I pointed out before, your loop limit should be: i < cantidad