マルチスレッド内のループについて

質問

こんにちは。
開発環境は　VC++6.0　SDI

マルチスレッドがあり、2つの処理を行う関数が書かれています。
このスレッドはダイアログボックスに配置したストップボタンを押すとフラグFALSEになりループを終了させます。

2つの関数は、int型の整数を引数にして、処理を行います。
整数はある値に達すると 0 になり永遠にループを続け、2つの処理を行います。
と言う意味合いでプログラムを書きました。(書いたつもりです)

(1)この書き方ですと、for内のループが動いている時に、右上の×ボタンでダイアログを閉じると
[Debug Assertion Failed!]と言う警告文が出て強制的に終了してしまいます。
ストップボタンを押してもcount=10になるまではループしています。（当然ですが・・。）
そもそもマルチスレッドの中にこのような形でfor文を入れるのは間違っているのでしょうか？

どのような書き方にすれば良いのでしょうか？
よろしくお願い致します。

bool m_flags;//スレッド内の処理を続けるか示すフラグ

UINT CabcDlg::thread(LPVOID pParam)//
{
CabcDlg *pInst = (CabcDlg *) pParam;

while(pInst->m_flags){//ストップボタンが押されると終了する。
for (int count = 0; count<10; count++ )
{
pInst->OnSend(count); //処理A
pInst->OnReceive(count);//処理B
}
}
return 0;
}

回答No.9

sha-girl · ベストアンサー

>GetWindowText、SetWindowTextの行が含まれていると
>WaitForSingleObject()でずっと止まってしまいます。
おそらく無効になった子ウインドウ(ここではエディトボックス)を呼び出しているのが原因だと思います。

気になった箇所としては
No4の回答の「回答に対する補足」に書かれている箇所ですが
CDialog::OnDestroy()が先頭で呼ばれています。

>void CabcDlg::OnDestroy()/*********追記************/
>{
>CDialog::OnDestroy();
>if (pThread != NULL) {
>WaitForSingleObject(*pThread,INFINITE);
>pThread=NULL;
>}
>}

CDialog::OnDestroy();ですが一番後ろで呼んでください。

もし上記で問題があれば
CDialogがどの時点で子ウインドウを廃棄するのかわかりませんが
BOOL CDialog::PreTranslateMessage(MSG* pMsg)をオーバーライドしてください。
×ボタンで閉じるとき
WM_COMMANDが呼ばれおそらくLOWORD(wParam)にIDOKかIDCANCELがはいっていると思います。

if (pMsg->message == WM_COMMAND ){
　if (LOWORD(pMsg->wParam) == IDOK || LOWORD(pMsg->wParam) == IDCANCEL){
　　//ここにスレッド待ちの処理をいれる
　}
}

お礼

返事が遅くなってしまい申し訳ございませんでした。

皆様のアドバイスのおかげで期待通りの動作になりました。
一人で考えていても絶対出来なかったと思います。
とても勉強になりました。ありがとうございました。

回答No.8

sha-girl

MrBanさんの指摘の通りpThreadがメンバなのかどうかよくわかりません。
CWinThread* m_pThreadをCabcDlgのメンバに持つなどして対策してください。

1.コンストラクタでm_pThreadの初期値をいれてください。
CabcDlg::CabcDlg():m_pThread(NULL)
{
}

2.AfxBeginThreadの戻り値をm_pThreadで持ちます。
m_pThread = AfxBeginThread(thread, this);としてください

3.WaitForSingleObject(m_pThread, INFINITE);ではなく
if (m_pThread != NULL){
if (m_pThread->m_hThread != NULL){
WaitForSingleObject(m_pThread->m_hThread, INFINITE);
m_pThread = NULL;
}
}
としてください。

ちなみに
WaitForSingleObject(*m_pThread, INFINITE);
でも可能ですが
何故可能なのか分かって使っていますか？
CWinThreadにoperator HANDLE() const;
が定義されているからです。
WaitForSingleObject(m_pThread, INFINITE);
に関しては使い方を間違っています。

お礼

sha-girlさんありがとうございます。

色々細かいところまで見て頂きとても勉強になります。
WaitForSingleObject()が働いていることが確認できました。（戻り値を確認しました）
本当にありがとうございます。

ただ、GetWindowText、SetWindowTextの行が含まれていると
WaitForSingleObject()でずっと止まってしまいます。
なのでこの件について色々調べている所です。
自分でもがんばって調べているのですが、
もし何か気づかれることがございましたらご指摘お願い致します。

//スレッド（MrBanさんの#7についての補足に書きました物をコピーしたものです）
UINT CabcDlg::thread(LPVOID pParam)//
{
CabcDlg *pInst = (CabcDlg *) pParam;
CString strData;

while(pInst->m_flags){//ストップボタンが押されると終了する。
if (count == 9){
count = 0;//ループのカウント数を0に戻す
}
/********処理A*********/
pInst->m_ComBox01[count].GetWindowText((CString) strData)/******この行があるとフリーズする***************/
pInst->m_serial->Send((LPCSTR)strData, strData.GetLength());//処理用のクラスに送信データを送る。
//補足(1)m_serial = シリアル通信用クラスへのポインタ

/********処理B**********/
処理B（エディットボックスにデータを書き込み）
data = pInst->m_serial->Receive();//処理用のクラスから受信データをもらってくる。
pInst->m_edit1[count].SetWindowText(data.pszBuf);/******この行があるとフリーズする***************/
//補足(2)data1.pszBuf = char pszBuf[256]ここに受信した文字列が入っています。
count++;//カウントアップ
}
return 0;
}

回答No.7

MrBan

> 以下のように宣言しています。
メンバの変数ですよね。この変数に値を入れるのは誰ですか。

> CWinThread *pThread = AfxBeginThread(thread, this);//スレッド開始
これを見ると、同名の別変数で起動しているようですが、
メンバ変数には格納されてますか。

> WaitForSingleObject(pThread, INFINITE);//エラー無しでダイアログを閉じることができる。
多分これはちゃんと利いてないのでは。
WaitForSingleObjectのエラー値をちゃんと確認してますか？

> WaitForSingleObject(*pThread, INFINITE);//フリーズしていまう。
スレッドが終わるのを待っていますが、
このメッセージハンドラ内でWaitしたら、メッセージキューも止まります。
こうなると、今のつくりではスレッドの方も止まってしまうのではないですか？(想像ですが)

情報が少なすぎて、これ以上はなんともいえませんが、
多分、OnSend/OnReceiveの直接呼出し/実装に問題がある気がします。

お礼

MrBanさんアドバイスありがとうございます。
また見難い記述であるにも関わらず読んでいただけてとても感謝しております。

補足

>メンバの変数ですよね。この変数に値を入れるのは誰ですか。
pThreadはメンバ変数なので、m_pThreadと訂正致します。
m_pThreadにはAfxBeginThread(thread, this)が値(スレッド オブジェクトへのポインタ)を入れます。

>> CWinThread *pThread = AfxBeginThread(thread, this);//スレッド開始
>これを見ると、同名の別変数で起動しているようですが、
>メンバ変数には格納されてますか。
一行で書いたり、分けて書いたり見難い書き方をしてしまい申し訳ございませんでした。
実際は分けて書いています。

>> WaitForSingleObject(pThread, INFINITE);//エラー無しでダイアログを閉じることができる。
>多分これはちゃんと利いてないのでは。
>WaitForSingleObjectのエラー値をちゃんと確認してますか？
sha-girlさんのアドバイスでWaitForSingleObjectは機能するようになりました。
戻り値はWAIT_OBJECT_0（指定したオブジェクトがシグナル状態になったことを意味します。）です。

WaitForSingleObject(*pThread, INFINITE);//フリーズしていまう。
このフリーズしてしまう箇所は色々調べた所スレッド内の処理で、
コンボボックスからデータを取得する箇所（GetWindowText）、
またエディットボックスからデータを取得する箇所（SetWindowText）のようです。
この行を // でなくすと100回近く実験しても正常に終了しました。

//スレッド
UINT CabcDlg::thread(LPVOID pParam)//
{
CabcDlg *pInst = (CabcDlg *) pParam;
CString strData;

while(pInst->m_flags){//ストップボタンが押されると終了する。
if (count == 9){
count = 0;//ループのカウント数を0に戻す
}
/********処理A*********/
pInst->m_ComBox01[count].GetWindowText((CString) strData)/******この行があるとフリーズする***************/
pInst->m_serial->Send((LPCSTR)strData, strData.GetLength());//処理用のクラスに送信データを送る。
//補足(1)m_serial = シリアル通信用クラスへのポインタ

/********処理B**********/
処理B（エディットボックスにデータを書き込み）
data = pInst->m_serial->Receive();//処理用のクラスから受信データをもらってくる。
pInst->m_edit1[count].SetWindowText(data.pszBuf);/******この行があるとフリーズする***************/
//補足(2)data1.pszBuf = char pszBuf[256]ここに受信した文字列が入っています。
count++;//カウントアップ
}
return 0;
}

回答No.6

MrBan

> 2つのスレッドが動かないように一行削除致しました。
> //m_hThread = pThread->m_hThread;/********スレッドの２重起動のため削除*****/
OnButton1で既に起動済みかを確認とかしないと、
本質的に、AfxBeginThreadが動いてしまい、二回起動しようとするのですが。

> これってボタン１を２回押すと２つスレッドが起動して
> m_hThreadが上書きされる可能性がありませんか？
というご指摘ですよ。m_hThread の更新だけの問題ではないです。

> ただ・・・。なんの処理が終わるのを待っているのか止まったまま動きません。

参照したサンプルは、おそらく起動したスレッドの関数が終了することを待ってます。

ですが、ソースを見る限りここでは「待っているつもり」なだけかも知れません。

OnDestroy時点の pThread に何が入っているか分かりません。
m_がないので、ローカルなのか。
AfxBeginThreadの戻り値をどこかでちゃんと保存しているのか。
いずれにしても、今出ているソースから想像するに多重起動すると最初のスレッドの情報が失われて終了が待てなくなると思われます。

補足

MrBanさんありがとうございます。

>> これってボタン１を２回押すと２つスレッドが起動して
>> m_hThreadが上書きされる可能性がありませんか？
>というご指摘ですよ。m_hThread の更新だけの問題ではないです。

勘違いをしておりました。下のように追記致しました。
void CabcDlg::OnButton1()
{
if(m_flags == true){ //フラグをチェックする。
m_flags = false; //フラグを0にする。
if (pThread != NULL) { //スレッドが動作していたら
WaitForSingleObject(pThread, INFINITE);//スレッドの終了を待って
pThread = NULL; //スレッドを終了する。
}
}

CWinThread *pThread = AfxBeginThread(thread, this);//スレッド開始

}

>OnDestroy時点の pThread に何が入っているか分かりません。
>m_がないので、ローカルなのか。
以下のように宣言しています。
class CabcDlg : public CDialog
{
private:
CWinThread *pThread;// RS232C受信スレッド
・
・

WaitForSingleObjectの第一引数を
AfxBeginThreadの戻り値と、CWinThreadの値で下の２通りで試した所、
WaitForSingleObject(pThread, INFINITE);//エラー無しでダイアログを閉じることができる。
WaitForSingleObject(*pThread, INFINITE);//フリーズしていまう。
という現象でした。

ただ、エラー無しでダイアログを閉じることが出来ると言うのも、何回か繰り返すとエラーとなるときがありました。
×で閉じたときにスレッドのどこを実行していたかによってこのような事が起きていると思います。
どこでエラーとなっているかもっと詳しく調べているところです。

WaitForSingleObject第一引数はどちらの方がよいのでしょうか？

回答No.5

MrBan

CabcDlg::~CabcDlg(void)
{
WaitForSingleObject(m_hThread,INFINITE);//スレッドがシグナル状態になるまで待機し続ける。
}

デストラクタに来た時点では遅いです。もう既にWindowは削除済みです。

回答No.4

sha-girl

>×が押された時点で、コンボボックスやエディットボックスへのハンドルが消滅してしまうのでしょうか？？
CDialogのソースを見ないと分かりませんが
CDialog::OnCloseあたりにそのあたりの事がかかれていると思います。
そこで子ウインドウが開放されているかもしれません。
CDialog::OnCloseをオーバーライドして、そこでスレッド終了を待つと良いかもしれません。
void CabcDlg::OnClose()
{
　if(m_hThread) WaitForSingleObject(m_hThread,INFINITE); // m_hThreadはコンストラクタでNULLをいれておいた方がよいです。
　CDialog::OnClose(); // 基底を呼ぶのを忘れずに
}

先の回答でも言いましたが、この作りで2つ以上のスレッドが同時に動くとまずいです。
もし２つ以上のスレッドが同時に動く可能性があるのならm_hThreadを配列にするか
vectorにする等して、同時に動くスレッド分のハンドルは保持しておく必要があります。

補足

sha-girlさんありがとうございます。

>この作りで2つ以上のスレッドが同時に動くとまずいです。
2つのスレッドが動かないように一行削除致しました。

>CDialog::OnCloseをオーバーライドして、そこでスレッド終了を待つと良いかもしれません。
OnClose内にブレークポイントを作り、スレッド動作中に×ボタンを押すとブレークポイントで止まらないままでエラーメッセージがでてそのまま終了してしまいました。

void CDialog::OnDestroy()を
オーバーライドして、ここにブレークポイントを作り、同じようにスレッド動作中に×ボタンを押したところ、ブレークポイントで止まりました。
ここに　WaitForSingleObject　を書いたところ処理が終わるまでまっているようになりました。
ただ・・・。なんの処理が終わるのを待っているのか止まったまま動きません。
処理の方を見直してみようと思います。

WaitForSingleObject(*pThread,INFINITE);の第一引数ですが、色々なQ&Aの過去ログを調べたところ、このように書かれている物がいくつかあったので
このようにしてみました。

CabcDlg::~CabcDlg(void)
{
//WaitForSingleObject(m_hThread,INFINITE);/*****削除******/
}

void CabcDlg::OnButton1()
{
CWinThread *pThread = AfxBeginThread(thread, this);//スレッド開始
//m_hThread = pThread->m_hThread;/********スレッドの２重起動のため削除*****/
}

void CabcDlg::OnDestroy()/*********追記************/
{
CDialog::OnDestroy();
if (pThread != NULL) {
WaitForSingleObject(*pThread,INFINITE);
pThread=NULL;
}
}

UINT CabcDlg::thread(LPVOID pParam)//
{
CabcDlg *pInst = (CabcDlg *) pParam;
CString strData;

while(pInst->m_flags){//ストップボタンが押されると終了する。
for (int count = 0; count<10; count++ )
{
/********処理A*********/
pInst->m_ComBox01[count].GetWindowText((CString) strData)//コンボボックスからデータを取得。
pInst->m_serial->Send((LPCSTR)strData, strData.GetLength());//処理用のクラスに送信データを送る。

/********処理B**********/
処理B（エディットボックスにデータを書き込み）
}
}
return 0;
}

回答No.3

sha-girl

CWinThread *pThread = AfxBeginThread(thread, this);//スレッド開始
m_hThread = pThread->m_hThread;
これってボタン１を２回押すと２つスレッドが起動してm_hThreadが上書きされる可能性がありませんか？

先の回答でも答えましたが
まずDebug Assertion Failedがでたとき「中止」ボタンを押してどこで止まってるのか調べてください。
更にその時点の呼び出し履歴もみると良いでしょう。

お礼

sha-girlさんアドバイスありがとうございます。

>CWinThread *pThread = AfxBeginThread(thread, this);//スレッド開始
>m_hThread = pThread->m_hThread;

何とかハンドルを取得しようとしたのですが、見当違いでした。

エラーとなる位置は、スレッド内の処理Aで、
送信用のデータをコンボボックスから入手しようとしている場所
処理Bで受信データをエディットボックスに書き込もうとしている場所でした。

×が押された時点で、コンボボックスやエディットボックスへのハンドルが消滅してしまうのでしょうか？？

回答No.2

MrBan

本質的に、OnSendとOnReceiveがCabcDlgのメンバである必要があるのか、提示の情報の範囲では疑問が残ります。

処理を見たわけではないので憶測に基づきますが、
これらの処理が、名前の示すように何かの送受信であれば、
「ダイアログ」の機能として適切なのかどうか…。
何でもかんでも情報がダイアログのメンバに詰め込んであるだけだったりはしませんか？

表示の更新などだけをDlg側に、できればPostMessage等で送り、
送受信の機能はそれようのクラスを作ってスレッド側で処理するのが安全ではないかと思います。

お礼

MrBanさんアドバイスありがとうございます。

OnSend(count);
OnReceive(count);
についてですが、

OnSend(count); では送信するデータをコンボボックスから入手しているだけで、実際の送信は、別のシリアル送受信用のクラスで行っています。
OnReceive(count);についても同様で、ここでは受信して得たデータをエディットボックスに書き込んでいるだけです。

スレッド内でコンボボックスやエディットボックスなどの情報を扱う方法を知らなくこのような回りくどい方法になってしまっています・・・。

回答No.1

sha-girl

スレッド内にfor文やwhile文をいれるのは問題ありません。
×ボタンを押すとCacbDlgのデストラクタが呼ばれると思いますが
その時にもまだスレッドが動いていたらやばいですよね？
（pParamで渡されたインスタンス(CabcDlg*)が廃棄されようとしているのに->OnSend等を呼んでるわけですから）
×ボタンを押した時点でスレッドを止め、スレッドが終了するまで待機してください。
(デストラクタの先頭でも良いかもしれません。)
スレッドが終了したかどうかを判断するには
スレッドのハンドルがシグナル状態かどうかで判断できます。
http://www.doumo.jp/postgretips/tips.jsp?tips=92

それでもまだ
[Debug Assertion Failed!]がでるようなら
そこで中止し、おそらくMFC内部で止まっていると思いますが
何故ASSERT(・・・)の箇所がFALSEになるのか調べてください。

お礼

sha-girlさんありがとうございます。

下記のようにプログラムを追加しました。
(1)HANDLE m_hThread;//スレッドへのハンドル
(2)デストラクタ内でのWaitForSingleObject(m_hThread,INFINITE);

それでもやはり[Debug Assertion Failed!]が出てしまいます。
この書き方ですと、スレッド中に×ボタンが押された時、デストラクタが呼び出されるが、スレッドが終了するまで
実行されないで待っている？となるのかな？？って思っていたのですが。
sha-girlさんのアドバイスが実行できていないのか、他に内部でエラーが起こっているのか・・。
もう少しがんばってみます。

class CabcDlg : public CDialog
{
private:
HANDLE m_hThread;//スレッドへのハンドル
bool m_flags;//スレッド内の処理を続けるか示すフラグ
}

CabcDlg::~CabcDlg(void)
{
WaitForSingleObject(m_hThread,INFINITE);//スレッドがシグナル状態になるまで待機し続ける。
}

void CabcDlg::OnButton1()
{
CWinThread *pThread = AfxBeginThread(thread, this);//スレッド開始
m_hThread = pThread->m_hThread;
}

UINT CabcDlg::thread(LPVOID pParam)//
{
CabcDlg *pInst = (CabcDlg *) pParam;

while(pInst->m_flags){//ストップボタンが押されると終了する。
for (int count = 0; count<10; count++ )
{
pInst->OnSend(count); //処理A
pInst->OnReceive(count);//処理B
}
}
return 0;
}

補足

MFCでAfxBeginThreadでスレッドを作った場合戻り値はハンドルではなく、ポインタなんですが、
同じ用途で扱えるのでしょうか？

デストラクタ内でSleep(5000)など十分な時間をかけてもエラーが出てしまう場合は他の点でエラーとなっていると判断する材料になりますでしょうか？

まだ解決することが出来ていません・・・。
よろしくお願い致します。