CnPack Forum - Powered by Discuz! Board

Subject: 转贴：有朋友报告的CnThreadPool.pas中可能有的问题。 [Print This Page]

Author: Passion Time: 2007-7-13 08:43 Subject: 转贴：有朋友报告的CnThreadPool.pas中可能有的问题。

1 、用TerminateThread过程会强制结束线程，这里会造成任务中来不及释放的对象内存泄漏，

2、

procedure TCnThreadPool.KillDeadThreads;

var

  i: Integer;

begin

  if csThreadManagment.TryEnter then

  try

for i := 0 to FThreads.Count - 1 do

   if TCnPoolingThread(FThreads[i]).IsDead then

   begin

      if FDeadTaskAsNew then

      begin

      if TCnPoolingThread(FThreads[i]).FProcessingDataObject <> nil then

         AddRequest(TCnPoolingThread(FThreads[i]).FProcessingDataObject, // 2、这里会造成递归调用，导致堆栈溢出。

            [cdQueue, cdProcessing]);

      //TCnPoolingThread(FThreads[i]).Terminate(True);

      TCnPoolingThread(FThreads[i]).Free;

      FThreads.Delete(i)

      end

      else

      begin

      TCnPoolingThread(FThreads[i]).Terminate(False);

      FThreadsKilling.Add(FThreads[i]);

      FThreads.Delete(i)

      end;

      try

      FThreads.Add(FThreadClass.Create(Self));

      except

{$IFDEF DEBUG}

      on E: Exception do

         Trace('New thread Exception on ' + E.ClassName + ': ' + E.Message)

{$ENDIF DEBUG}

      end

   end

  finally

csThreadManagment.Leave

  end

end;

Author: shenloqi Time: 2007-7-13 16:01

1.TerminateThread过程应该本来就是在出现问题的时候强制结束线程的，所以是会出现可能的问题，不过应该是有属性可以选择不强制结束线程的。

2.这里只是发现有线程已经死了的时候，回收线程的时候根据属性判断是不是需要将被回收的线程正在处理的数据重新放到队列中，只有可能某任务一直处理不了，就会被一直放到队列中，始终占用一个线程，但是并不会递归调用啊，而且回收的时候还判断了正在处理的和队列中是否有重复的对象，如果有重复的对象是不会再放到队列中的。

我看了一下代码，这里似乎的确有问题，可能是测试不够。KillDeadThreads会调用AddRequest，而AddRequest又会调用IncreaseThreads，InCreaseThreads又会调用KillDeadThreads，虽然KillDeadThreads的确会在调用AddRequest之后删掉对象，以便下一次不会重复添加，不过实际上递归在删除之前就发生了。而且还有另外一个隐含的的问题，就是检查重复的时候也检查了正在处理的对象，不过却没有管线程是不是已经死了，所以即使不会递归，也会造成实际上对象还是不能够被重新处理。

要解决这个问题，一种方法是给TCnTaskDataObject添加一个CreateFrom的虚构造函数，根据传入的对象复制一份对象，这样在AddRequest之前就可以复制一份对象后删除原来的线程以防止递归，或者是先从FThreads中删除被认为死掉的线程，再处理，这样也应该可以解决这个问题。

我待会儿更新一下这个文件，但是没有合适的测试用例，希望能够改对。

Author: Passion Time: 2007-7-13 16:03

好的。沈兄辛苦了。要是没法子提交CVS就发给我我来提交。

Author: shenloqi Time: 2007-7-13 21:36

提交好了：
修改了DeadTaskAsNew会导致无限递归和不生效的BUG，如果需要使用DeadTaskAsNew则需要实现TCnTaskDataObject.Clone方法（参见例子，例子也已经更新）；增加了一个TCnThreadPool.AddRequests方法。

Author: shenloqi Time: 2007-7-15 21:34

由于在AddRequest时Free线程有时候会引起阻塞，所以在上次的修复中针对待杀线程队列做了一些处理，使得AddRequest不再阻塞，我觉得可能还是需要一个专门的线程来负责清理问题比较好，不过这样的话线程池就会比较重型了，起码每个线程池要多一个线程（也可以不管多少个线程池都只有一个释放线程，但是可能引发释放不及时的问题）。

Welcome to CnPack Forum (http://bbs.cnpack.org/)