スレッド実行中の状態が取得できない

[戻る]
かれー 2010/04/23(金) 11:59:09
お世話になっております。

前回こちらで質問させて頂いた内容をもとに、
マルチスレッドの勉強を行っているのですが
度々で申し訳ありませんがご質問させてください。


※ここに載せるために若干内容を変更してありますので
※動作しないなどの、コード上の間違いがあるかもしれません

// グローバル変数
public:
    CWinThread*    hThread
    BOOL        stopFlag;

----------------------------
void Test::BtnClick()
{
    DWORD    dwExitCode;

    if(GetExitCodeThread( hThread, &dwExitCode ))
    {
        if( dwExitCode == STILL_ACTIVE ){
            stopFlag = TRUE;
        }
    };

    stopFlag = FALSE;
    hThread = AfxBeginThread(WorkThread, this);
}

UINT WorkThread(LPVOID pParam)
{
    Test* pDlg = (Test*)pParam;
    
    int i =1;
    while(i){
        i++;
        if( pDlg->stopFlag ) break;
    }
    return TRUE;
}


一度目にBtnClick()でスレッドを動かし、
二度目のBtnClick()に入ったあとに
GetExitCodeThread() で状態を取得して
一度目のスレッドを終了し、二度目のスレッドを開始させようと思ったのですが
GetExitCodeThread()が何故かFALSEを返してしまいます。

スレッドが動いてる点までは確認でき、hThread にも値が入ってる事を確認しました。
なぜGetExitCodeThreadが失敗してしまうのでしょうか?


何か記述的に問題があるのでしょうか?
提示する情報が足りなかったらご指摘ください。

PATIO 2010/04/23(金) 13:12:02
CWinThread*をスレッドハンドルと混同しているからではないかと思います。
CWinThread*はあくまでもCWinThreadクラスのインスタンスを指すポインタです。
GetExitCodeThreadが要求しているのはHANDLEですよね。
明らかに型が違うと思いますよ。
MSDNでCWinThreadを調べてみるべきだと思います。

maru 2010/04/23(金) 13:12:03
> なぜGetExitCodeThreadが失敗してしまうのでしょうか?
http://msdn.microsoft.com/ja-jp/library/cc429118.aspx
より
「関数が失敗すると、0 が返ります。拡張エラー情報を取得するには、 関数を使いま
す。」
あれ、関数名が落ちてる。GetLastError関数を使ってください。

BOOL GetExitCodeThread(
  HANDLE hThread,      // スレッドのハンドル
  LPDWORD lpExitCode   // 終了ステータス
);
に対して
   CWinThread*    hThread
ですが、コンパイルエラーになってません?
少なくとも、
    CWinThread*    pThread;
    GetExitCodeThread(pThread->m_hThread);
    pThread = AfxBeginThread(WorkThread, this);
となっている必要があるのでは?

仲澤@失業者 2010/04/23(金) 13:13:26
if( GetExitCodeThread( hThread->m_hThread, &dwExitCode))
  :

ではどうですか?。

maru 2010/04/23(金) 13:30:06
ついでに
> 一度目のスレッドを終了し、二度目のスレッドを開始させようと思ったのですが
フラグ操作で一度目のスレッドを終了させようとしているようですが、スレッドが
終了するのはスレッド関数が、フラグをチェックしたあとです。
このコードではすぐ後にフラグを落としていますので、スレッド関数側が確実に
ループを抜けるかどうか分かりません。スレッドが終了した後のフラグを落として
二つ目のスレッド起動しないとまずいでしょう。

> UINT WorkThread(LPVOID pParam)
> {
/* 中略 */
>     return TRUE;
> }
UINTだって言っているのに return TRUE;は無いんじゃないの。
確かに TRUE は UINT といってもエラーにはならないけど。

maru 2010/04/23(金) 13:31:57
修正。orz

> スレッドが終了した後のフラグを落として
スレッドが終了したのを確認した後にフラグを落として

maru 2010/04/23(金) 13:38:30
あぁ、それから
>    BOOL        stopFlag;
は  volatile BOOL stopFlag; としておかないとreleaseビルドではスレッドが
終了しないからね。

かれー 2010/04/23(金) 14:41:54
>PATIOさん、maruさん、仲澤@失業者さん
ご指摘のとおり、型が間違っておりました。
hThread を hThread->m_hThread とする事で解決できました。

CWinThread をMSDNで調べてみたのですが、これはオブジェクトなんですね。
GetExitCodeThread( hThread, &dwExitCode ) のままでも何故かコンパイルが
通ってしまうので、HANDLE なんだと勘違いしておりました。

ありがとうございました。



>maruさん
ご指摘ありがとうございます。

>UINTだって言っているのに return TRUE;は無いんじゃないの。
この場合、return 1; のように記述すべきということでしょうか?

>volatile BOOL stopFlag; としておかないとreleaseビルドではスレッドが〜
volatile の意味を調べてみたのですが、これを変数に付ける事によって
コンパイラの最適化を無視して変数が勝手に書き換えられる事が無い、
という解釈でよろしかったでしょうか?

maru 2010/04/23(金) 16:04:46
> GetExitCodeThread( hThread, &dwExitCode ) のままでも何故かコンパイルが
> 通ってしまうので、HANDLE なんだと勘違いしておりました。
typedef void *HANDLE;
だから、 HANDLE の代わりにポインタを渡してもエラーにならないんだね。
警告位は出なかったかな?

> この場合、return 1; のように記述すべきということでしょうか?
TRUEをどういう意味で使ったかによります。
TRUEは 1 と定義されているんだから 1 を使うところで TRUE と書いても問題ない
かもしれませんが、私としては TRUE は BOOL 型のところでのみ使うべきだろうと
いう思いです。実際のところ大きな問題ではないんでしょうけどね。

> volatile の意味を調べてみたのですが、これを変数に付ける事によって
> コンパイラの最適化を無視して変数が勝手に書き換えられる事が無い、
そのまんま「揮発性の」という意味ですね。
この変数は揮発性であり、値が毎回変わる可能性がある。したがって、最適化によっ
て変数の読出しを省略してはならない。ということです。
このプログラムではwhile文のスコープで stopFlag が変更されません。
そのため、最適化によってループ処理が省略される場合があります。

> 変数が勝手に書き換えられる事が無い
意味不明?

ryo 2010/04/23(金) 23:36:47
>> 変数が勝手に書き換えられる事が無い
>意味不明?
アプリの動作中に変数の値が保持されるって意味ではなく

最適化によって
「省略される」
「変数が定数に置き換わる」
「変数が書き換えられる」
とかいろんな言い方の違いだけじゃないかな

かれー 2010/04/26(月) 09:33:19
返信遅くなってしまいすみません。

>maruさん
>警告位は出なかったかな?
警告すら出ませんでした・・・


>TRUEをどういう意味で使ったかによります。
ネットで見つけたサンプルコードなどを参考に作成したものなので、
特別意味を持たせて使ってはいません。

戻り値を特に使っているわけでもありませんし、
値をFALSE、TRUEのどちらにしても私の作成したコードでは
動作に違いはないようです(見えない所であるのかも)

この戻り値に値を設定する意味が良くわかっていないのですが、
特別使う事が無ければなんでも良いという事なんでしょうか?



>このプログラムではwhile文のスコープで stopFlag が変更されません。
>そのため、最適化によってループ処理が省略される場合があります。
なるほど、変数が書きられる、と言うことではなく
変数が書き換えられた事を認識できない、といった意味だったのですね。

毎回変数を読みに行くと思っていたのですが、ループの中では
コンパイラが勝手に最適化して変数を読みにいってくれなくなるのですね。

PATIO 2010/04/26(月) 09:40:37
VC++を使ってプログラミングをするのであれば、
Cで始まり、ハンガリアン記法で命名されている型名はクラスである
可能性が高いと考えた方がいいですよ。
自分が知らない型に関してはMSDN等で確認を取るのは勿論ですが、
Microsoftが作成している部分に関してはある程度法則性があるので
名前を見ただけでもある程度の検討はつけられます。

volatileに関しては最適化の影響を受けないようにする為ですね。
最適化によってソース上の記述から直接変換されたコードとは
異なるが、動作としては同じなるように書き換えられますが、
最適化時にすべてのソースの関係を把握して最適化を行なって
いるわけではない為に好ましくない書き換えが行なわれる場合が
あるのでそういう変数が最適化の影響を受けないようにするのが目的です。
最適化による書き換えは状況によって幾つかパターンがあるので
一概にどれとは言えないでしょうけれど。

PATIO 2010/04/26(月) 09:48:57
> この戻り値に値を設定する意味が良くわかっていないのですが、
> 特別使う事が無ければなんでも良いという事なんでしょうか?

プログラムの動作と言う面だけを考えると値さえあっていれば
動作しますから問題はないと思います。
こういう部分の話と言うのは主にメンテナンス性の部分が大きいと
思います。TRUEやFALSEという定数を使うとTRUEとFALSEで意味がある
様に見えます。後々ソースを読み返した時にTRUEってどういう意味?
と考えてしまうので混乱しやすくなると言う意味です。
逆にTRUEである事、FALSEである事に意味があるならこれを使う事で
ソースの可読性があがります。
数値としての1がほしいのか?真偽としての1がほしいのかで
ソース上のわかりやすい記述は変わりますよねという事だと思います。

一般に真偽値としての返却をしたい場合は関数の型もBOOLを使いますから
返却値の型がUINTなら1でいいんじゃないかと言う話だと思います。
関数の型がUINTなのはフレームワーク上の関係でそうなっているだけで
この関数に関してはTRUEが正しいんだと言う場合はTRUEでも良いわけですけどね。
この場合は関数の返却としてTRUEだったらこう言う事ですよと
コメントを入れておくと後からソースを見た時にわかりやすくなります。

subaru 2010/04/26(月) 10:05:17
>この戻り値に値を設定する意味が良くわかっていないのですが、
>特別使う事が無ければなんでも良いという事なんでしょうか?

はい。
プロセスの終了コードと同様好きなように使ってかまいません。

スレッドを終了したいのであれば、GetExitCodeThreadなんかは必要なくて
単にstopFlagを設定してスレッドを待機すればよいだけです。

かれー 2010/04/26(月) 16:37:38
>PATIOさん
アドバイスありがとうございます。
知らない型にはついては逐一MSDNなどでチェックする癖をつけたほうが良さそうですね。

今回は戻り値に関しては、特に使用していないので
return 0; としておこうと思います。
(1やTRUEだと何か意味があるように見えてしまうので)


>subaruさん
>スレッドを終了したいのであれば、GetExitCodeThreadなんかは必要なくて
>単にstopFlagを設定してスレッドを待機すればよいだけです。
GetExitCodeThread を使わない良い方法があるのでしょうか?
現状は以下のようなコードを作成して、問題なく動いていると思いますが
どうもゴリ押しで動かしてるような気がして、もう少しスマートにならないかな・・・と
考えています。


※InitDialogで初期化
※初期化しておかないとGetExitCodeThreadで止まるようで・・・
hThread =NULL;

----------
if(hThread)
{
    GetExitCodeThread( hThread->m_hThread, &dwExitCode );
    if( dwExitCode == STILL_ACTIVE ){
        stopFlag = TRUE;
    }
}

if( stopFlag ){
    // 停止フラグON:スレッドの終了を確認して開始
    while(stopFlag)
    {    
        if(GetExitCodeThread( hThread->m_hThread, &dwExitCode )){
            if( dwExitCode != STILL_ACTIVE ){
                stopFlag = FALSE;
                hThread = AfxBeginThread(WorkThread, this);
            }
        }else{
            stopFlag = FALSE;
            hThread = AfxBeginThread(WorkThread, this);
        }
    }
}
else{
    // 停止フラグOFF:そのままマルチスレッド開始
    hThread = AfxBeginThread(WorkThread, this);
}

subaru 2010/04/26(月) 17:24:02
>GetExitCodeThread を使わない良い方法があるのでしょうか?
MFCだとどうするのがよいのかちょっとわかりませんが、
一応WaitForSingleObjectで終了を待ことができます。

stopFlagがTRUEのときにスレッドが中断されるように作ってあるから
stopFlagをTRUEにしてから終了待機すればよいということです。

tetrapod 2010/04/27(火) 09:27:44
提示サンプルは m_bAutoDelete=TRUE のままスレッドが走り出す構造だから
スレッド終了時点で pThread は自動的に delete されてしまう。
その後は pThread->m_hHandle を取得できなくなるので、うまくいかなくて当たり前。

かれー 2010/04/27(火) 11:48:38
[[解決]]
以下の形で作成し、問題なく動作するようになりました。
とりあえず、この件は解決としておきます。
みなさんありがとうございました。

if(hThread)
{
    stopFlag = TRUE;
    ::WaitForSingleObject(hThread->m_hThread, INFINITE);

    stopFlag = FALSE;
    hThread = AfxBeginThread(WorkThread, this);
}
else{
    hThread = AfxBeginThread(WorkThread, this);
}


>subaruさん
WaitForSingleObject を使う事で大分すっきりした形にする事ができました。
ありがとうございます。

>tetrapodさん
この作りですと、hThread がスレッド終了時に delete されてても、
WaitForSingleObject が WAIT_FAILED を返すだけですので、
動作的には問題ないかなと思い m_bAutoDelete=FALSE は省略しました。

tetrapod 2010/04/27(火) 12:47:44
> この作りですと、hThread がスレッド終了時に delete されてても、
> WaitForSingleObject が WAIT_FAILED を返すだけですので、
んなことはない。そもそも WaitFor... までたどり着けない可能性がある。
hThread が delete 済みである場合、その hThread を使って
hThread->m_hThread とするだけでアクセス違反になる可能性がある。
今アクセス違反になっていないのはたまたまと考える必要がある。

# hThread と書くとスレッドハンドルに見えるので書き方がよくない (指摘済み)
# どうせ書くなら pThread だろう (スレッドオブジェクトへのポインタなので)

かれー 2010/04/27(火) 13:57:52
なるほど、今の状態はたまたま上手くいってるだけだったのですね
変数のわかりにくさもご指摘ありがとうございます。
上でも書かれていたようですが、直接ではなかったので気づいていませんでした。

最終的に手直しして、以下のようにしました。
このままだとスレッド実行が一度だけの場合、delete が出来ていない設計なので、
デストラクタあたりに追加するのが普通なのでしょうか?



※InitDialogで初期化
hThread =NULL;

----------

if(pThread)
{
    stopFlag = TRUE;
    ::WaitForSingleObject(pThread->m_hThread, INFINITE);
    delete pThread;
    pThread = NULL;
}

// スレッド実行
stopFlag = FALSE;
pThread = AfxBeginThread(WorkThread, this, THREAD_PRIORITY_NORMAL, 0, 
CREATE_SUSPENDED);
pThread->m_bAutoDelete = FALSE;
pThread->ResumeThread();

かれー 2010/04/27(火) 14:07:30
※追記
デストラクタに delete を追加した場合、
スレッドがまだ動いてる状態で delete されるとアクセス違反になるようです・・・
もう少し考えて見ます。

ryo 2010/04/27(火) 14:34:45
デストラクタにdeleteだけじゃなく

if(pThread)
{
    stopFlag = TRUE;
    ::WaitForSingleObject(pThread->m_hThread, INFINITE);
    delete pThread;
    pThread = NULL;
}
これ全部追加すればいい

subaru 2010/04/27(火) 14:42:39
ウインドウに関連した処理とかの場合はOnDestroyの方がいいかも。
ウインドウが破棄されるタイミングと
デストラクタが呼ばれるタイミングは同じではないので。

かれー 2010/04/27(火) 15:42:23
>ryoさん
なるほど、そもそも全部追加すればよかったわけですね。
簡単な事でした・・・

>subaruさん
OnDestroy に記述して正常に呼ばれていました。
デストラクタの呼ばれるタイミングの違いについては、また別途勉強してみます。

古いスレッドには追加発言できません。