[C++] Come trovare 'sto bug?

[sottovento]

Ciao a tutti. Sembra che qualcuno si diverta sottovento, e mi servirebbe il vostro aiuto.

Ho una semplice classe con alcuni membri, fra i quali ce n'e' uno statico, qualcosa del tipo:


class MyClass
{
private:
int a;
int b;
....
static int counter;
public:
MyClass (int a);
~MyClass ();
....
}


La cosa bella e' che l'autore dell'opera ha pensato in INCREMENTARE il contatore nel costruttore e DECREMENTARLO nel distruttore.
L'idea dell'autore (penso) era quello di avere un contatore del numero di oggetti attualmente nel sistema (contatore ovviamente inizializzato a zero).
Questa e' la classe base degli oggetti di cui si vuole sapere il numero presente in memoria. Tutte le classi, quindi, derivano da questa.

Dopo qualche ora di funzionamento, il contatore e' negativo. e devo trovarne il motivo.

La prima cosa che ho pensato e' stata quella di definire il costruttore di copia e l'assegnamento: siccome il software e' piuttosto grosso (qui ho semplificato), ho pensato di definirli privati in questa classe base e ricompilare tutto il mestiere.
Risultato: si compila correttamente. Sono sconcertato.
A questo punto, mi sembra che si possano fare due ipotesi:
1 - ho sbagliato a ridefinire costruttore di copia & assegnamento
2 - non ho considerato qualche altra opzione.

Considerando che il contatore e' privato e che viene effettivamente incrementato solo da costruttore e distruttore (ho fatto la prova), cosa potrei controllare ancora?

Cheers

[ilsensine]

Ambiente multithread?
doppio delete da qualche parte?

[sottovento]

Ambiente multithread?
doppio delete da qualche parte?

L'ambiente e' sicuramente multithread e non posso escludere il doppio delete, considerando che ho visto anche qualche crash (anche se sono rari, circa un paio di volte al mese. Me ne aspetterei di piu' in questo caso, visto l'applicativo).
E' un ottimo suggerimento, anche se speravo di evitare di dover controllare tutto il codice. Grazie.

Se viene in mente qualcos'altro, io sono sempre qui :D ....

[ilsensine]

L'ambiente e' sicuramente multithread
Allora proteggi con un veloce mutex il contatore (se stai in ambiente SMP o se compili in debug). Se risolve, ti porrai il problema di eliminare il mutex e usare le operazioni atomiche appropriate.
e non posso escludere il doppio delete, considerando che ho visto anche qualche crash
sostituisci ~MyClass () con virtual ~MyClass (), così un doppio delete ha maggiori probabilità di andare in crash velocemente (ed essere beccato).

Se viene in mente qualcos'altro, io sono sempre qui :D ....
La lista è lunga, soprattutto visto che sei in ambiente multithread...

[sottovento]

Allora proteggi con un veloce mutex il contatore (se stai in ambiente SMP o se compili in debug). Se risolve, ti porrai il problema di eliminare il mutex e usare le operazioni atomiche appropriate.

Si, era gia' protetto. Almeno questo me l'aspettavo.


sostituisci ~MyClass () con virtual ~MyClass (), così un doppio delete ha maggiori probabilità di andare in crash velocemente (ed essere beccato).

Anche questo era gia' stato fatto, fortunatamente....


La lista è lunga, soprattutto visto che sei in ambiente multithread...
Non preoccuparti, qualsiasi cosa ti venga in mente e' benvenuta. Cmq grazie, non avevo pensato alle cose piu' semplici. Come sempre

[ilsensine]

Qualche puntatore selvaggio potrebbe scrivere sopre la variabile counter...

[ilsensine]

Puoi anche provare con:

class MyClass
{
private:
....
static int counter;
std::list<MyClass *> objects;
...

MyClass::MyClass (int a)
{
LockMutex();
assert(counter==objects.size());
objects.push_back(this);
counter++;
UnlockMutex();
}

MyClass::~MyClass ()
{
LockMutex();
assert(counter==objects.size());
objects.remove(this);
counter--;
assert(counter==objects.size()); // Doppio delete!
UnlockMutex();
}

Armati di debugger e attendi...

[sottovento]

Puoi anche provare con:

class MyClass
{
private:
....
static int counter;
std::list<MyClass *> objects;
...

MyClass::MyClass (int a)
{
LockMutex();
assert(counter==objects.size());
objects.push_back(this);
counter++;
UnlockMutex();
}

MyClass::~MyClass ()
{
LockMutex();
assert(counter==objects.size());
objects.remove(this);
counter--;
assert(counter==objects.size()); // Doppio delete!
UnlockMutex();
}

Armati di debugger e attendi...

Si, capisco cosa intendi (by the way: immagino che anche objects debba essere static), ma non sono sicuro che possa portarmi facilmente ad una soluzione. Cmq grazie, e' sempre un buon aiuto.

A questo punto, invece che memorizzare i puntatori agli oggetti, potrei mettere un contatore locale, decrementato ad ogni distruttore (doppio delete)....

[ilsensine]

(by the way: immagino che anche objects debba essere static)
Sì certo scusa

A questo punto, invece che memorizzare i puntatori agli oggetti, potrei mettere un contatore locale, decrementato ad ogni distruttore (doppio delete)....
Il fatto è che non sono propriamente sicuro che sia un doppio delete. Non so che compilatore stai usando, ma almeno sul gcc un doppio delete su una classe con distruttore virtuale è un segfault al 100%

[sottovento]

Sì certo scusa


Il fatto è che non sono propriamente sicuro che sia un doppio delete. Non so che compilatore stai usando, ma almeno sul gcc un doppio delete su una classe con distruttore virtuale è un segfault al 100%

No, il software in questione e' scritto con Visual Studio.
Naturalmente non si puo' essere sicuri che sia un doppio delete, anzi probabilmente non lo e'. Comunque in casi come questo (montagne di software scritto da decine di persone di aziende diverse, stili di programmazione tutti diversi, ...) e' bene controllare anche l'ovvio.
Immagino che anche su Visual si possa avere un bel crash in caso di doppio delete, ma la cosa non mi turba. Non e' sicuramente una perdita di tempo.

Thanks