删除指针有时会导致堆损坏

Question

我有一个使用自定义线程池类运行的多线程应用程序。线程都执行相同的函数，但参数不同。

这些参数通过以下方式提供给线程池类：

// 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);

一旦线程无事可做，它就获取下一个参数并运行作业函数。我决定负责删除线程池类中的参数：

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

但是，这样做时，我有时会收到堆损坏错误：

指定给 RtlFreeHeap 的地址无效

奇怪的是，在一种情况下它可以完美运行，但在另一个程序中它会因为这个错误而崩溃。我尝试在其他地方删除指针：在作业函数执行后的线程中（我得到相同的堆损坏错误）或在作业函数本身的末尾（在这种情况下没有错误）。

我不明白从不同的地方删除相同的指针（我检查过，地址是相同的）会改变什么。这与它是多线程的事实有什么关系吗？

我确实有一个关键部分来处理对参数的访问。我认为问题不在于同步访问。无论如何，只有在所有线程完成后才会调用析构函数，并且我不会在其他任何地方删除任何指针。指针可以自动删除吗？

至于我的代码。作业列表是一个结构的队列，由作业的id（用于以后能够获取特定作业的输出）和参数组成。

getNextJob() 由线程调用（它们有一个指向 ThreadPool 的指针）每次完成执行最后一个作业时。

void ThreadPool::addJob(int jobId, void* params) {
    jobData job; // jobData is a simple struct { int, void* }
    job.ID = jobId;
    job.params = params;

    // insert parameters in the list
    this->jobs.push(job);
}

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

    // we don't want to start a same job twice,
    // so we make sure that we are only one at a time in this part
    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;
}

Answer 1

让我们换个思路：你为什么要使用指针？

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);
  }
}

没有 new、delete 或 pointers... 显然一些实现细节可能会被歪曲，但您可以了解整体情况。

Answer 2

感谢您提供额外的代码。现在我们可以看到一个问题 -

在getNextJob中

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

在“pop”之后，“job”指向的内存是undefined。不要使用参考，复制实际数据！

试试这样的（它仍然是通用的，因为 JobData 是通用的）：

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;

}

此外，当您将作业添加到队列时，您还必须锁定互斥锁，以防止列表损坏。 AFAIK std::lists 本质上不是线程安全的......？

Answer 3

根据规范对指向 void 的指针使用运算符 delete 会导致未定义的行为。

C++ 规范草案的第 5.3.5 章。第 3 段。

在第一种选择（删除对象）中，如果操作数的静态类型与其动态类型不同，则静态类型应为操作数动态类型的基类，并且静态类型应具有虚拟析构函数或行为未定义。在第二种选择（删除数组）中，如果要删除的对象的动态类型与其静态类型不同，则行为未定义。73)

以及相应的脚注。

这意味着不能使用 void* 类型的指针删除对象，因为没有 void 类型的对象

Answer 4

对作业队列的所有访问都必须同步，即通过在访问之前锁定作业队列，一次仅从 1 个线程执行。您是否已经有一个临界区或一些类似的模式来保护共享资源？同步问题通常会导致难以重现的奇怪行为和错误。

Answer 5

用这么多的代码很难给出明确的答案。但一般来说，多线程编程就是同步访问可能从多个线程访问的数据。如果没有长或其他同步原语保护对线程池类本身的访问，那么您可能有多个线程同时到达您的删除循环，此时您几乎可以保证双重释放内存。

在作业函数结束时删除作业参数时没有崩溃的原因可能是因为对单个作业参数的访问已经被工作队列隐式序列化。或者你可能只是走运了。无论哪种情况，最好将锁和同步原语视为不是保护代码的东西，而是保护数据的东西（我一直认为这个术语“关键部分”在这里有点误导，因为它往往会导致人们想到“代码行部分”而不是数据访问）。在这种情况下，因为您想从多线程，你需要通过锁或其他同步原语来保护它。

Answer 6

如果你尝试删除一个对象两次，第二次就会失败，因为堆已经被释放了。这是正常行为。

现在，由于您处于多线程上下文中......可能是删除“几乎”并行完成，这可能会避免第二次删除时出错，因为第一次删除尚未完成。

Answer 7

使用smart pointers 或其他RAII 来处理您的记忆。

---

如果您有权访问 boost 或 tr1 库，您可以执行以下操作。

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));
}