2017-01-10 6 views
-1

std :: bindで作成されたstd ::関数を受け入れるヘルパークラスを作成したいので、このクラスを再描画することができますstd :: functionとスレッドC++ 11との組み合わせでベクトル内のデバッグアサーションが失敗する

短い例:別のスレッドからdebug assertion Failed , see Image: i.stack.imgur.com/aR9hP.png:停止を呼び出すとき

void loopme() { 
    std::cout << "yay"; 
} 

main() { 
    LoopThread loop = { std::bind(&loopme) }; 
    loop.start(); 
    //wait 1 second 
    loop.stop(); 
    //be happy about output 
} 

ただし、()私の現在の実装では、次のエラーが発生します。

エラーがスローされる理由は誰にも分かりますか? この例ではベクトルを使用していません。 スレッド内からloopmeを呼び出すのではなく、std :: coutに直接出力すると、エラーはスローされません。

ここに私のクラスの完全な実装:

class LoopThread { 
public: 
    LoopThread(std::function<void(LoopThread*, uint32_t)> function) : function_{ function }, thread_{ nullptr }, is_running_{ false }, counter_{ 0 } {}; 
    ~LoopThread(); 
    void start(); 
    void stop(); 
    bool isRunning() { return is_running_; }; 
private: 
    std::function<void(LoopThread*, uint32_t)> function_; 
    std::thread* thread_; 
    bool is_running_; 
    uint32_t counter_; 
    void executeLoop(); 
}; 

LoopThread::~LoopThread() { 
    if (isRunning()) { 
     stop(); 
    } 
} 

void LoopThread::start() { 
    if (is_running_) { 
     throw std::runtime_error("Thread is already running"); 
    } 
    if (thread_ != nullptr) { 
     throw std::runtime_error("Thread is not stopped yet"); 
    } 
    is_running_ = true; 
    thread_ = new std::thread{ &LoopThread::executeLoop, this }; 
} 

void LoopThread::stop() { 
    if (!is_running_) { 
     throw std::runtime_error("Thread is already stopped"); 
    } 
    is_running_ = false; 
    thread_->detach(); 
} 

void LoopThread::executeLoop() { 
    while (is_running_) { 
     function_(this, counter_); 
     ++counter_; 
    } 
    if (!is_running_) { 
     std::cout << "end"; 
    } 
    //delete thread_; 
    //thread_ = nullptr; 
} 

私は(ただし、コードを含む単純なmainメソッドが動作するはずです)テストのために、次のGoogletestコードを使用:

void testfunction(pft::LoopThread*, uint32_t i) { 
    std::cout << i << ' '; 
} 

TEST(pfFiles, TestLoop) 
{ 
    pft::LoopThread loop{ std::bind(&testfunction, std::placeholders::_1, std::placeholders::_2) }; 
    loop.start(); 

    std::this_thread::sleep_for(std::chrono::milliseconds(500)); 

    loop.stop(); 
    std::this_thread::sleep_for(std::chrono::milliseconds(2500)); 
    std::cout << "Why does this fail"; 

} 
+0

ただし、引用符で囲まれたコードはありません。重要な部分は省略しましたか、またはあなたはベクターを明示的に使用しないと言っていますか? –

+0

以前はGoogle Testを使っていませんでしたが、あなたのTEST()定義には何らかのASSERT()宣言が含まれているはずですか? – Mitch

+1

offtopic: 'std :: thread'は動かすことができます。ポインタでそれを保存して手動で削除する必要はありません。 –

答えて

4

のご利用is_running_は、定義されていない動作です。これは、1つのスレッドで書き込みを行い、同期バリアなしで別のスレッドで読み込むためです。

部分的にこれにより、stop()は何も停止しません。このUBがなくても(つまり、アトミックを使って "修正"する)、 "oy、ある時点で停止する"と言いますが、結局はストップが起きたことを保証しようとしません。

コードはnewを不必要に呼び出します。ここにstd::thread*を使用する理由はありません。

あなたのコードは5というルールに違反しています。あなたはデストラクタを書いた後、無視されたコピー/移動操作をしました。それはばかげて壊れやすい。

stop()は、スレッドを停止することは何もありません。thisへのポインタを持つスレッドは、LoopThreadオブジェクトよりも寿命が長くなります。 LoopThreadは範囲外になり、std::threadが格納するポインタを破棄します。実行中のexecuteLoopは、破棄されたstd::functionを呼び出し、無効なメモリにカウンタをインクリメントします(別の変数が作成されたスタック上にある可能性があります)。

大雑把に言えば、コードの3〜5行(インターフェイスの宣言を数えない)ごとにスレッドを使用してstdスレッドを使用すると、1つの基本的なエラーがあります。

技術的なエラー以外にも、設計は間違っています。 detachを使用することは、ほとんど常に恐ろしい考えです。あなたがスレッド終了時に準備をして、その約束のどこかで完了するのを待って、あなたのプログラムをきれいで信頼できるシャットダウンのようなものを得ることが不可能に近いという約束がない限り、

推測しておいて、ベクトルエラーは、スタックメモリ全体をスムーズにして、ほとんど無効なポインタの後に実行する関数を見つけるためです。テストシステムは、あなたがゴミを出している場所に配列インデックスを置き、デバッグvectorは範囲外であることをキャッチします。あるいは、std関数の実行が半分になるような関数ポインタです。


スレッド間でのみ同期されたデータで通信します。これは、データ、またはあなたが馬鹿馬鹿しくしていない限り、mutexが守られていることを意味します。あなたは空想を得るのに十分なスレッドを理解していません。あなたは気の利いた人をコピーして適切に使用するのに十分なスレッドを理解していません。空想を得ないでください。

newを使用しないでください。ほとんどの場合、newを使用しないでください。絶対にする必要がある場合は、make_sharedまたはmake_uniqueを使用してください。しかし、あまり使用しないでください。

detachスレッドを使用しないでください。期間。はい、これはループが完了するのを待たなければならないかもしれないということです。それを処理するか、シャットダウン時などに待機するスレッドマネージャを作成します。

どのスレッドがどのデータを所有しているかを明確にしてください。スレッドがデータで終了したときを非常に明確にしてください。スレッド間で共有されるデータは使用しないでください。値(または不変の共有データへのポインタ)を渡して通信し、std::futureから情報を取得します。


プログラム方法の学習には多くのハードルがあります。あなたがこれまでに得たことがあれば、あなたはいくつか合格しました。しかし、あなたはおそらく、あなたの側に沿って学んだ人たちが、以前のハードルの一つに落ちたことを知っています。

  • シーケンス、それは次々と起こります。

  • フロー制御。

  • サブプロシージャと関数。

  • ルーピング。

  • 再帰。

  • ポインタ/参照と動的vs自動割り当て。

  • 動的ライフタイム管理。

  • オブジェクトと動的ディスパッチ。

  • 複雑

  • スペース

  • メッセージ

  • スレッディングと同時実行

  • 不均一なアドレス空間、シリアライズと

  • 関数型プログラミング、メタネットワーキング座標関数、カリング、部分適用、モナド

このリストは完全ではありません。

これらのハードルのそれぞれが、プログラマーとしてクラッシュして失敗する可能性があり、これらのハードルを正しく取得するのは難しいです。

スレッディングが難しいです。それは簡単な方法です。動的なライフタイム管理は難しい。それは簡単な方法です。どちらの場合も、非常に賢い人が「手作業」でそれを行う方法をマスターしています。その結果、予測できない/未定義の動作がランダムに発生し、多くのプログラムがクラッシュします。手動リソースの割り当てと割り当て解除とマルチスレッドコードを混乱させることができますが、結果は通常、小さなプログラムが誤って動作する(気づいたバグを修正した場合にのみ動作します)。そして、あなたがそれを習得すると、最初の習得は、プログラム全体の「状態」を頭に入れ、それがどのように働くかを理解するという形でもたらされます。これは大規模な多数の開発者向けコードベースに対応できないため、偶然、偶然に大量のプログラムを卒業することになります。

make_uniqueの両方のスタイルと唯一の変更不可能な共有データベースのスレッドが可能な戦略です。これは、小さな部分が正しい場合にそれらをまとめると、結果のプログラムが正しいことを意味します(リソースの存続期間と並行性に関して)。これにより、小規模なスレッド化またはリソース管理のローカルな習得が、これらの戦略が機能するドメイン内のラルフスケールプログラムに適用されるようになります。

+0

はこの50 upvotesを与えるでしょう –

+0

フィードバック、私はそれに取り組み、私の結果を私の将来の質問を掲載する予定です。 次の行を読むと、私は合計でポイントを逃したと思うかもしれません。あなたは警告されています:) クラスにはいくつかの問題がありますが、私は同期化されていないis_running_を使用すると多くのダメージを与える可能性があることを知っていました。 しかし、私のexpections(C + +の初心者として)私は500ミリ秒beforの睡眠と停止後2秒の睡眠は、機能の最後のアクセスは、メインスレッドは範囲外になります。 ただし、コードを修正する時間は –

+0

@ mac.1です。同期せずにデータを読み取ると、未定義の動作になります。この場合、解放時に、読み取りスレッドは読み取り結果をキャッシュします。読んだものとの同期がないことを知っていたので、一度読むことができました。**修正するための明確な方法がなかったため、**再び見たことはありません**。このスレッドでは発生しませんでしたが、同期がない場合は、他のスレッドで発生する可能性を無視することができます。 UBは、コンパイラが自由にそれが起こらないと想定し、起こらないかのように最適化することを意味します。 – Yakk

0

は、私は私のプログラムの開発を再構築することを決め@Yakkからガイドに従った後:

  1. bool is_running_td::atomic<bool> is_running_
  2. stop()に変更されます停止をトリガーするだけでなく、activly経由を停止するスレッドを待ちますthread_->join()
  3. newのすべてのコールが、私はコピーや移動コンストラクタ経験がないstd::make_unique<std::thread>(&LoopThread::executeLoop, this)
  4. に置き換えられます。だから私はそれらを禁じることにした。これは私が誤ってこれを使用するのを防ぐはずです。私はいつか将来、それらを必要とする場合、私はこれは、リストの最後にある

(2を参照)thread_->join()に置き換えたthoose

  • thread_->detach()上deepter外観を取らなければなりません。

    class LoopThread { 
    public: 
        LoopThread(std::function<void(LoopThread*, uint32_t)> function) : function_{ function }, is_running_{ false }, counter_{ 0 } {}; 
        LoopThread(LoopThread &&) = delete; 
        LoopThread(const LoopThread &) = delete; 
        LoopThread& operator=(const LoopThread&) = delete; 
        LoopThread& operator=(LoopThread&&) = delete; 
        ~LoopThread(); 
        void start(); 
        void stop(); 
        bool isRunning() const { return is_running_; }; 
    private: 
        std::function<void(LoopThread*, uint32_t)> function_; 
        std::unique_ptr<std::thread> thread_; 
        std::atomic<bool> is_running_; 
        uint32_t counter_; 
        void executeLoop(); 
    }; 
    
        LoopThread::~LoopThread() { 
        if (isRunning()) { 
         stop(); 
        } 
    } 
    
    void LoopThread::start() { 
        if (is_running_) { 
         throw std::runtime_error("Thread is already running"); 
        } 
        if (thread_ != nullptr) { 
         throw std::runtime_error("Thread is not stopped yet"); 
        } 
        is_running_ = true; 
        thread_ = std::make_unique<std::thread>(&LoopThread::executeLoop, this); 
    } 
    
    void LoopThread::stop() { 
        if (!is_running_) { 
         throw std::runtime_error("Thread is already stopped"); 
        } 
        is_running_ = false; 
        thread_->join(); 
        thread_ = nullptr; 
    } 
    
    void LoopThread::executeLoop() { 
        while (is_running_) { 
         function_(this, counter_); 
         ++counter_; 
        } 
    } 
    
    TEST(pfThread, TestLoop) 
    { 
        pft::LoopThread loop{ std::bind(&testFunction, std::placeholders::_1, std::placeholders::_2) }; 
        loop.start(); 
        std::this_thread::sleep_for(std::chrono::milliseconds(50)); 
        loop.stop(); 
    } 
    
  • 関連する問題