线程代码反馈意见

下面是一些代码，有人想在生产应用程序中使用（不是我，诚实） - 我想请一些独立的反馈意见。线程代码反馈意见

public class JobProcessor<TJob> : IJobProcessor<TJob> where TJob : class 
{ 
    private readonly IJobQueue<TJob> theJobQueue = 
    new NullQueue<TJob>(); 

    private Thread processorThread; 

    private bool shutdownRequested; 

    private readonly IJobObserver<TJob> theObserver = new NullObserver<TJob>(); 

    public AutoResetEvent threadStartedEvent = new AutoResetEvent(false); 

    private int processorThreadId = 0; 

    private volatile TJob currentJob = null; 

    public JobProcessor(IJobQueue<TJob> jobQueue, IJobObserver<TJob> observer) 
    { 
    if (observer != null) 
    { 
    theObserver = observer; 
    } 
    shutdownRequested = false; 
    theJobQueue = jobQueue; 
    CreateAndRunThread(); 
    } 

    private void CreateAndRunThread() 
    { 
    processorThread = new Thread(Run) 
         { 
         Name = "Tpk Processor Thread", IsBackground = true 
         }; 
    processorThread.SetApartmentState(ApartmentState.STA); 
    processorThread.Priority = ThreadPriority.BelowNormal; 
    processorThread.Start(); 
    } 

    public void Shutdown() 
    { 
    threadStartedEvent.WaitOne(); 

    shutdownRequested = true; 

    theJobQueue.Interrupt(processorThreadId); 
    } 

    public TJob CurrentJob() 
    { 
    return currentJob; 
    } 

    private void Run() 
    { 
    processorThreadId = Thread.CurrentThread.ManagedThreadId; 

    threadStartedEvent.Set(); 

    while (!BufferClearedAndShutDown()) 
    { 
    try 
    { 
    ProcessNextMessage(); 
    } 
    catch (ThreadAbortException) 
    { 
    CreateAndRunThread(); 
    break; 
    } 
    catch (Exception e) 
    { } 
    } 
    } 

    private void ProcessNextMessage() 
    { 
    currentJob = theJobQueue.RetrieveJob(); 
    if (currentJob != null) 
    { 
    theObserver.ProcessMessage(this, currentJob); 
    } 
    currentJob = null; 
    } 

    private bool BufferClearedAndShutDown() 
    { 
    return theJobQueue.IsEmpty && shutdownRequested; 
    } 
} 
}

来源

2009-02-14 Peter Morris

请删除不必要的代码注释;他们只会分散注意力，让阅读变得更难。 – 2009-02-14 19:51:15

你线程试图抓住ThreadAbortException然后重新创建自己吗？我不知道这是可能的，但无论如何，这不是一个好的方式来玩操作系统。

如果在currentJob = theJobQueue.RetrieveJob（）后发生异常，您将失去作业。但在Observer.ProcessMessage（this，currentJob）之前;

而且除非您的jobQueue是线程安全的，否则应该在访问它时添加锁定。

来源

2009-02-14 19:46:51

很难给你不知道IJobQueue，IJobObserver，或IJobProcessor的语义有用的反馈，但这里是站出来几个细节：似乎

processorThread并不像真正需要它;可以/应该只是在CreateAndRunThread本地
shutdownRequested应标明挥发性，shutdownReqeusted设置为true后调用Thread.MemoryBarrier（），或使用Thread.VolatileWrite设置shutdownRequested场
为什么等待线程启动要求它关机之前？
不知道为什么你需要一个threadStartedEvent，它用于什么？特别是使它公开是有点可怕
不知道如何IJobQueue.Interrupt实施，很难说如果有问题或不
谁使用CurrentJob，为什么？感觉有风险
捕获自己的ThreadAbortException会捕获大多数情况，它不会捕获所有内容。您可以使用单独的监视器线程，它调用Thread.Join，并在双重检查BufferClearedAndShutdown（）后调用CreateAndRunThread（）

来源

2009-02-14 20:17:44

这仅仅是一个生产者/消费者队列吗？有很多预先推出的例子 - 我前几天自己在这里发布了一个（但我目前找不到它）......这样说 - 我发布的版本是很多更简单 - 即“显然没有错误“，而不是”没有明显的错误“。我去看看我能找到它......

（编辑：found it）

的一点是，在该版本中，工作线程只是做：

T item; 
while(queue.TryDequeue(out item)) { 
    // process item 
} 
// queue has been closed and drained

来源

2009-02-14 20:24:10

那里有很多方面从代码所缺少的，但ID只是想补充我的2个便士以上，这点我同意。

在兰迪的第2项，我只想用一个锁声明，而不是记忆障碍，这是MSDN推荐的方法，除非你使用多安腾CPU对您的箱子（具有弱的存储排序）

http://msdn.microsoft.com/en-us/library/system.threading.thread.memorybarrier.aspx

我完全同意第6项。公开的方法暴露了工作队列中物品的参考？那么它是不是很好呢？尤其是当你讨论线程安全性的时候，如果你需要这个类的一些信息暴露给外部世界，那么它就是不可变的，并且暴露它。更好的封装。

来源

2009-02-15 00:01:31 Matt

现在有些人已经回答了，我觉得它是安全的，添加自己的评论没有任何偏置:-)

你为什么要等待线程关闭它之前启动？
记录线程ID并将其用作识别哪个线程可以关闭它的方法？
currentJob标记为volatile但未关闭请求（并且它未设置在锁内）
为什么在一个队列中混合一个队列+ worker？为什么不分开一个队列，然后使用一个或多个线程来处理它。
我想不出为什么他会期待ThreadAbortException，以便消除当前工作线程并创建一个新线程。
尽管关于如何简单地扩展类以利用多个工作线程的代码顶部发表了评论，但我不认为这种设计使得移动到多个工作线程非常容易（与队列中不同的方法不同/工人是分开的）。

我不是多线程的大师或任何东西，所以我想发布它只是为了确保它不仅仅是我认为这段代码看起来有点狡猾。

来源

2009-02-15 18:31:20

线程代码反馈意见

回答

相关问题