Löschen von Zeiger führt manchmal zu Heap-Beschädigung

Question

Ich habe eine Multithread-Anwendung, die mit einer benutzerdefinierten Thread-Pool-Klasse ausgeführt wird. Die Threads führen alle die gleiche Funktion mit unterschiedlichen Parametern aus.

Diese Parameter werden an den Threadpool-Klasse mit dem folgenden Weg:

// jobParams is a struct of int, double, etc... 
jobParams* params = new jobParams; 
params.value1 = 2; 
params.value2 = 3; 

int jobId = 0; 

threadPool.addJob(jobId, params); 

Sobald ein Thread nichts zu tun hat, ist es die nächsten Parameter erhält und betreibt die Jobfunktion. Ich entschied, Sorge für die Löschung der Parameter in der Threadpool Klasse zu nehmen:

ThreadPool::~ThreadPool() { 
    for (int i = 0; i < this->jobs.size(); ++i) { 
     delete this->jobs[i].params; 
    } 
} 

aber wenn dies zu tun, habe ich manchmal einen Heapbeschädigung Fehler:

Invalid Address specified to RtlFreeHeap

Das Merkwürdige ist, dass in In einem Fall funktioniert es perfekt, aber in einem anderen Programm stürzt es mit diesem Fehler ab. Ich habe versucht, den Zeiger an anderen Stellen zu löschen: im Thread nach der Ausführung der Job-Funktion (ich bekomme den gleichen Heap-Fehler) oder am Ende der Job-Funktion selbst (kein Fehler in diesem Fall).

Ich verstehe nicht, wie das Löschen der gleichen Zeiger (ich überprüfte, die Adressen sind die gleichen) von verschiedenen Orten ändert nichts. Hat das irgendetwas damit zu tun, dass es Multithread ist?

Ich habe einen kritischen Abschnitt, der den Zugriff auf die Parameter behandelt. Ich glaube nicht, dass das Problem synchronisierter Zugriff ist. Wie auch immer, der Destruktor wird nur aufgerufen, sobald alle Threads fertig sind, und ich lösche keinen anderen Zeiger irgendwo anders. Kann Zeiger automatisch gelöscht werden?

Wie für meinen Code. Die Liste der Jobs ist eine Queue einer Struktur, bestehend aus der ID eines Jobs (um später die Ausgabe eines bestimmten Jobs abrufen zu können) und den Parametern.

getNextJob() wird von den Threads (sie haben einen Zeiger auf den ThreadPool) jedes Mal aufgerufen, wenn sie ihren letzten Job ausgeführt haben.

Answer 1

Lassen Sie uns das auf den Kopf stellen: Warum verwenden Sie überhaupt Zeiger?

class Params 
{ 
int value1, value2; // etc... 
} 

class ThreadJob 
{ 
    int jobID; // or whatever... 
    Params params; 
} 

class ThreadPool 
{ 
    std::list<ThreadJob> jobs; 

    void addJob(int job, const Params & p) 
    { 
    ThreadJob j(job, p); 
    jobs.push_back(j); 
    } 
} 

Keine neuen, löschen oder Zeiger ... Offensichtlich einige der Implementierungsdetails gespannt werden kann, aber Sie bekommen das Gesamtbild.

Answer 2

Der gesamte Zugriff auf die Jobwarteschlange muss synchronisiert werden, d. H. Nur von 1 Thread gleichzeitig ausgeführt werden, indem die Jobwarteschlange vor dem Zugriff gesperrt wird. Haben Sie bereits einen kritischen Abschnitt oder ein ähnliches Muster, um die freigegebene Ressource zu schützen? Synchronisationsprobleme führen oft zu merkwürdigem Verhalten und Fehlern, die schwer zu reproduzieren sind.

Answer 3

Mit dieser Menge an Code ist es schwierig, eine definitive Antwort zu geben. Im Allgemeinen geht es bei Multithread-Programmierung jedoch um die Synchronisierung des Zugriffs auf Daten, auf die von mehreren Threads aus zugegriffen werden kann. Wenn es kein langes oder anderes Synchronisations-Primitiv gibt, das den Zugriff auf die Threadpool-Klasse selbst schützt, können Sie möglicherweise mehrere Threads gleichzeitig zu Ihrer Löschschleife haben, wodurch Sie praktisch doppelt so viel Speicher freigeben können.

Der Grund dafür, dass Sie beim Löschen der Parameter eines Jobs am Ende der Jobfunktion keinen Absturz bekommen, liegt möglicherweise daran, dass der Zugriff auf die Params eines einzelnen Jobs bereits implizit von Ihrer Arbeitswarteschlange serialisiert wird. Oder du hast vielleicht nur Glück. In jedem Fall ist es am besten, über Sperren und Synchronisation Primitive als nicht etwas zu denken, das schützt Code, sondern als etwas, das schützt Daten (Ich habe immer gedacht, der Begriff "kritischer Abschnitt" war ein bisschen irreführend hier, da es dazu führt, dass die Leute eher an einen "Abschnitt von Codezeilen" als an Datenzugriff denken.Da Sie in diesem Fall auf Ihre Auftragsdaten aus mehreren Threads zugreifen möchten, müssen Sie sie über eine Sperre oder ein anderes Synchronisationsprimitiv schützen.

Answer 4

Wenn Sie versuchen, ein Objekt zweimal zu löschen, schlägt das zweite Mal fehl, weil der Heap bereits freigegeben wurde. Dies ist das normale Verhalten.

Nun, da Sie in einem Multithreading-Kontext sind ... könnte es sein, dass die Löschungen "fast" parallel durchgeführt werden, was den Fehler beim zweiten Löschen vermeiden könnte, da der erste noch nicht abgeschlossen ist.

Answer 5

Danke für zusätzlichen Code. Jetzt können wir sehen, ein Problem -

in getNextJob

if (!this->jobs.empty()) 
{ 
    job = &(this->jobs.front()); 
    this->jobs.pop(); 

Nach dem "Pop", der Speicher von 'job' spitz ist undefined. Verwenden Sie keine Referenz, kopieren Sie die eigentlichen Daten!

versuchen, etwas wie folgt aus (es ist immer noch generisch, weil Jobdata generisch):

jobData ThreadPool::getNextJob() // get the data of the next job 
{ 
    jobData job; 

    WaitForSingleObject(this->mutex, INFINITE); 

    if (!this->jobs.empty()) 
    { 
    job = (this->jobs.front()); 
    this->jobs.pop(); 
    } 

    // we're done with the exclusive part ! 
    ReleaseMutex(this->mutex); 

    return job; 

}

Auch während Sie Aufträge an die Warteschlange sind das Hinzufügen müssen Sie auch den Mutex sperren, zu verhindern Sie die Korruption der Liste. AFAIK std :: lists sind NICHT von Natur aus threadsicher ...?

Answer 6

Verwenden Sie smart pointers oder andere RAII, um mit Ihrem Speicher umzugehen.

---

Wenn Sie Zugriff auf Boost oder Tr1-Lib haben, können Sie so etwas tun.

class ThreadPool 
{ 
    typedef pair<int, function<void (void)> > Job; 
    list<Job> jobList; 
    HANDLE mutex; 

public: 
    void addJob(int jobid, const function<void (void)>& job) { 
     jobList.push_back(make_pair(jobid, job)); 
    } 

    Job getNextJob() {  

     struct MutexLocker { 
      HANDLE& mutex; 
      MutexLocker(HANDLE& mutex) : mutex(mutex){ 
       WaitForSingleObject(mutex, INFINITE); 
      } 
      ~MutexLocker() { 
       ReleaseMutex(mutex); 
      } 
     }; 

     Job job = make_pair(-1, function<void (void)>()); 
     const MutexLocker locker(this->mutex); 
     if (!this->jobList.empty()) { 
      job = this->jobList.front(); 
      this->jobList.pop(); 
     } 
     return job; 
    } 
}; 


void workWithDouble(double value); 
void workWithInt(int value); 
void workWithValues(int, double); 

void test() { 
    ThreadPool pool; 
    //... 
    pool.addJob(0, bind(&workWithDouble, 0.1)); 
    pool.addJob(1, bind(&workWithInt, 1)); 
    pool.addJob(2, bind(&workWithValues, 1, 0.1)); 
} 

Answer 7

Die Verwendung des Operators delete on pointer führt zu einem ungültigen Ergebnis gemäß der Spezifikation.

Kapitel 5.3.5 des Entwurfs der C++ - Spezifikation. Absatz 3.

In the first alternative (delete object), if the static type of the operand is different from its dynamic type, the static type shall be a base class of the operand’s dynamic type and the static type shall have a virtual destructor or the behavior is undefined. In the second alternative (delete array) if the dynamic type of the object to be deleted differs from its static type, the behavior is undefined.73)

Und entsprechende Fußnote.

This implies that an object cannot be deleted using a pointer of type void* because there are no objects of type void