Segmentation Fault en destuctor C++

Question

Un saludo.

Estoy haciendo un ejercicio corto y que en principio no debería dar más problemas, pero mis fieles amigos los segmentation fault ya están aquí. El programa trata de la gestión de un blog. Lo que a nosotros nos interesa es la relación de composición entre una entrada y sus comentarios, que son punteros dentro de un array, que a su vez es un atributo de la entrada:

#ifndef ENTRADA_H
#define ENTRADA_H
#include <string>
#include <stdexcept>
#include "Comentario.h"
class Entrada {
public:
    Entrada();
    Entrada(std::string titulo, std::string fecha, std::string texto);
    Entrada(const Entrada& orig);
    virtual ~Entrada();
    void addComentario(Comentario *comentario);
    void eliminarComentario(int cual);
    std::string getTexto();
    std::string getTitulo();
    std::string getFecha();
    int getNumComentarios();
    Entrada& setTexto(std::string &texto);
    Entrada& setTitulo(std::string &titulo);
    Entrada& setFecha(std::string &fecha);
    Comentario& getComentario (int cual);
    static const int MAX_COMENTARIOS = 30;
private:
    std::string _titulo = "";
    std::string _texto = "";
    std::string _fecha = "";
    int _numComentarios = 0;
    Comentario *_comentarios[MAX_COMENTARIOS];
};

#endif /* ENTRADA_H */

Como es lógico, si se elimina una entrada los comentarios mueren con ella, no siguen existiendo (es una relación de composición). Pues bien, el destructor de la entrada queda tal que así:

Entrada::~Entrada() {
    for (int i = 0; i < _numComentarios; i++){ //Con la entrada se destruyen los comentarios
        delete _comentarios[i];
    }
}

_comentarios[] se rellena en el propio main utilizando el método addCometnario de la clase Entrada:

void Entrada::addComentario(Comentario* comentario) {
    if (_numComentarios == MAX_COMENTARIOS)
        throw std::string ("Entrada::addComentario: No caben mas comentarios");

    _comentarios[_numComentarios] = comentario;
    _numComentarios++;
}

En el main queda tal que así:

Entrada entrada1 ("Bienvenida", "1-1-2010", "Bienvenidos al nuevo blog");
Comentario com1 ("AAA", "Hola!!!", "3-1-2010");
entrada1.addComentario(&com1);

Después de lo cual hay algunos cout y algunos try-catch que sí se comportan como se espera.

¿El problema? Al llegar al return 0 e ir a finalizar el programa, cuando se llama a los destructores de los objetos automáticos, es cuando se produce el segmentation fault, concretamente al ir a destruirse la entrada. El bucle for se ejecuta una vez (hay un comentario), y en esa ejecución se produce el segmentation fault al llamar al destructor de Comentario, que está por defecto, sin nada especial (en esa clase los atributos son objetos std::string sin arrays ni nada "especial").

Ahora me estoy planteando la hipótesis de que esto se producía porque se destruía antes el comentario que la entrada (o al revés), llamándose siempre a un destructor sin objeto que destruir, con lo cual he probado a crearlos manualmente con new y a borrarlos también manualmente con delete y, en efecto, ya no se produce el error (obviamente solo hago delete entrada1, pues ahí ya se elimina el comentario también):

    Entrada *entrada1 = new Entrada ("Bienvenida", "1-1-2010", "Bienvenidos al nuevo blog");
    Comentario *com1 = new Comentario ("AAA", "Hola!!!", "3-1-2010");
    entrada1->addComentario(com1);
    //Resto del main sin mayor importancia
    delete entrada1;
    //delete com1 no se pone, pues ya lo ha eliminado el destructor de la entrada
    return 0;

Pero claro, esto ya me lleva a plantearme: ¿es imposible hacerlo con objetos automáticos, al no poder controlar la llamada a los destructores?

Gracias. Saludos.

Answer 1

Un segmentatoion fault puede darse por varios motivos:

• Acceder a una zona de memoria que no pertenece a la memoria asignada por el programa.
  • Por ejemplo: Leer/Escribir más allá del límite de un búfer.
• Acceder a una zona de memoria que ya fue liberada.
  • Por ejemplo: Borrar dos veces una zona de memoria.
• Escribir en una zona de memoria marcada como sólo lectura.

Para evitar estos problemas se debe tener mucho cuidado con la memoria solicitada y su ciclo de vida; en el código que compartes aparentemente lo estás teniendo. Pero podrías ahorrarte todas las precauciones si siguieses ciertos principios de programación:

Define correctamente qué objetos son responsables de la memoria.

Tu objeto Entrada es conceptualmente incorrecto pues se hace cargo de memoria que no le pertenece: Se le facilita un puntero a datos cuyo origen desconoce, tendrás problemas si se le pasa un puntero a memoria que no es dinámica, un puntero nulo, un puntero incorrecto, un puntero a memoria ya liberada o un puntero a un tipo incorrecto:

Entrada e;
Comentario c("", "", "");
auto p = new Comentario("", "", "");
delete p;

e.addComentario(new Comentario ("AAA", "Hola!!!", "3-1-2010")); // Vale.
e.addComentario(&c);                     // Oh, oh...
e.addComentario(nullptr);                // ¡Repámpanos!
e.addComentario((Comentario *)0xfabada); // ¡Rayos y centellas!
e.addComentario(p);                      // ¡Maldición!
e.addComentario((Comentario *)&e);       // ¡Apocalipsis!

Para evitar semejantes disparates, si un objeto es responsable de liberar memoria, también debería ser responsable de crearla, por lo tanto la función Entrada::addComentario NO debería recibir un puntero a Comentario si no los datos necesarios para crear uno:

void Entrada::addComentario(const std::string &a, const std::string &b, const std::string &c) {
    if (_numComentarios == MAX_COMENTARIOS)
        throw std::string ("Entrada::addComentario: No caben mas comentarios");

    _comentarios[_numComentarios] = new Comentario(a, b, c);
    _numComentarios++;
}

Si además, no fuese necesario gestionar objetos Comentario fuera de Entrada, el primer objeto debería ser un tipo privado del segundo:

class Entrada {
public:
    // ...
    void addComentario(const std::string &a, const std::string &b, const std::string &c);
    // ...
private:
    struct Comentario { ... };
    // ...
    Comentario *_comentarios[MAX_COMENTARIOS];
};

Mejor aún, no gestiones memoria dinámica.

Tu código no parece un caso en que la memoria dinámica sea la solución, al fin y al cabo la cantidad de comentarios está prefijada ¿Para qué crearlos dinámicamente?

class Entrada {
public:
    // ...
    void addComentario(const std::string &a, const std::string &b, const std::string &c)
    {
        if (_numComentarios == MAX_COMENTARIOS)
            throw std::string ("Entrada::addComentario: No caben mas comentarios");

        _comentarios[_numComentarios].a = a;
        _comentarios[_numComentarios].b = b;
        _comentarios[_numComentarios].c = c;
        _numComentarios++;
    }
    // ...
private:
    struct Comentario { ... };
    // Colección de comentarios, NO DE PUNTEROS A comentario.
    Comentario _comentarios[MAX_COMENTARIOS];
};

Con esto desaparece la necesidad de destructor.

Otras cosas a tener en cuenta.

• Evita identificadores que empiecen por guiones bajos (_), lee este hilo para saber por qué (aunque ya se te mencionó en otra pregunta tuya).
• Los destructores virtuales son para llamar correctamente a los destructores de clases base cuando se deriva de ellas, si no vas a derivar nada de Entrada, no hay necesidad de que el destructor sea virtual (aunque ni siquiera tienes necesidad de destructor).

• Las funciones miembro que leen datos son conceptualmente constantes, márcalas como tal:

  class Entrada {
  public:
      // ...
      std::string getTexto() const;  // Sólo lectura.
      std::string getTitulo() const; // Sólo lectura.
      std::string getFecha() const;  // Sólo lectura.
      int getNumComentarios();
      // ...
      Comentario& getComentario (int cual) const;   // Sólo lectura.
      // ...
  };