2017-09-04 18 views
1

ワーカー[スレッド]クラスにサスペンド/レジューム機能を追加しようとして、私は説明できない問題が発生しました。 (C++ 1y/VS2015)ワーカースレッドの中断/再開の実装

問題はデッドロックのように見えますが、デバッガが接続され、ブレークポイントが特定のポイント(#1を参照)より前に設定されていると再現できないようですそれはタイミングの問題です。

私が見つけることができる修正(#2)は、ミューテックスを長く保つ必要があり、クライアントコードが他のミューテックスを取得しようとする可能性があるため、私には分かりませんデッドロックの可能性を高めます。

しかし、問題を修正します。

ワーカーループ:

Job* job; 
while (true) 
{ 
    { 
    std::unique_lock<std::mutex> lock(m_jobsMutex); 
    m_workSemaphore.Wait(lock); 

    if (m_jobs.empty() && m_finishing) 
    { 
     break; 
    } 

    // Take the next job 
    ASSERT(!m_jobs.empty()); 
    job = m_jobs.front(); 
    m_jobs.pop_front(); 
    } 

    bool done = false; 
    bool wasSuspended = false; 
    do 
    { 
    // #2 
    { // Removing this extra scoping seemingly fixes the issue BUT 
     // incurs us holding on to m_suspendMutex while the job is Process()ing, 
     // which might 1, be lengthy, 2, acquire other locks. 
     std::unique_lock<std::mutex> lock(m_suspendMutex); 
     if (m_isSuspended && !wasSuspended) 
     { 
     job->Suspend(); 
     } 
     wasSuspended = m_isSuspended; 

     m_suspendCv.wait(lock, [this] { 
     return !m_isSuspended; 
     }); 

     if (wasSuspended && !m_isSuspended) 
     { 
     job->Resume(); 
     } 
     wasSuspended = m_isSuspended; 
    } 

    done = job->Process(); 
    } 
    while (!done); 
} 

サスペンド/レジュームだけです:

void Worker::Suspend() 
{ 
    std::unique_lock<std::mutex> lock(m_suspendMutex); 
    ASSERT(!m_isSuspended); 
    m_isSuspended = true; 
} 

void Worker::Resume() 
{ 
    { 
    std::unique_lock<std::mutex> lock(m_suspendMutex); 
    ASSERT(m_isSuspended); 
    m_isSuspended = false; 
    } 
    m_suspendCv.notify_one(); // notify_all() doesn't work either. 
} 

(Visual Studioの)テスト:

struct Job: Worker::Job 
    { 
    int durationMs = 25; 
    int chunks = 40; 
    int executed = 0; 

    bool Process() 
    { 
     auto now = std::chrono::system_clock::now(); 
     auto until = now + std::chrono::milliseconds(durationMs); 
     while (std::chrono::system_clock::now() < until) 
     { /* busy, busy */ 
     } 

     ++executed; 
     return executed < chunks; 
    } 

    void Suspend() { /* nothing here */ } 
    void Resume() { /* nothing here */ } 
    }; 

    auto worker = std::make_unique<Worker>(); 

    Job j; 
    worker->Enqueue(j); 

    std::this_thread::sleep_for(std::chrono::milliseconds(j.durationMs)); // Wait at least one chunk. 

    worker->Suspend(); 

    Assert::IsTrue(j.executed < j.chunks); // We've suspended before we finished. 
    const int testExec = j.executed; 

    std::this_thread::sleep_for(std::chrono::milliseconds(j.durationMs * 4)); 

    Assert::IsTrue(j.executed == testExec); // We haven't moved on. 

    // #1 
    worker->Resume(); // Breaking before this call means that I won't see the issue. 
    worker->Finalize(); 

    Assert::IsTrue(j.executed == j.chunks); // Now we've finished. 

私が間違っている/何をしないのですか?なぜジョブのプロセス()はsuspendミューテックスによって保護されなければならないのですか?

EDITResume()は、通知時にミューテックスを保持していてはなりません。それは修正されました - 問題は解決しません。

答えて

0

もちろん、ジョブのProcess()は、suspendミューテックスによって保護される必要はありません。

j.executedのアクセス - アサートとインクリメントの場合 - ただし、同期化する必要があります(std::atomic<int>にするか、またはミューテックスなどでガードする)。

私はメインスレッドの変数に書き込んでいないので、なぜ問題が現れたのかはまだ分かりません。undefined behaviour propagating backwards in timeの場合があります。

関連する問題