CreateSolidBrushによるリソースリーク

[戻る]
vcshoshinsha 2009/10/09(金) 14:26:11
CreateSolidBrushによるブラシの作成に関して、過去スレでいくらか議論されています
が、いまいち解決できません。
私の場合、OnCtlColorで状態をチェックし、色を変更します。
そのいろの変更は任意に変更できなければならないので、HBRUSHのメンバ変数をもち、
ダイアログ等のコンストラクタでCreateSolidBrushを一度のみににする、という方法が
とれません。CreateSolidBrushを必要とするタイミングが無限あるわけです。
そのような場合、DeleteObjectする必要があるのですが、OnCtlColorの段階ではまだ消
せません。
そうなるとどのタイミングでDeleteObjectすればよいのでしょうか?
どなたかご教授願います。

maru 2009/10/09(金) 16:35:02
> そのような場合、DeleteObjectする必要があるのですが、OnCtlColorの段階ではまだ> せません。
OnCtlColorでCreateSolidBrushしているが、そのブラシを使用するのは別の所なので、
CreateSolidBrushしている場所ではDeleteObjectできないということでしょうか?

それでしたら、OnCtlColorではフラグを立てるだけにして、ブラシを使用する前にその
フラグを検査して、フラグが立っていたら、ブラシを作ればよろしいのでは。

vcshoshinsha 2009/10/09(金) 17:05:53
すみません。
OnCtlColorが描画前というのはわかるのですが、それ以降、どのようにMFCで処理がなさ
れていくのかがわかりません。
ブラシが使われるタイミングというのはどのタイミングなのでしょうか?
その関数かなにかをオーバーライドかなにかすればできるのでしょうか?

ITO 2009/10/09(金) 17:17:57
ん?
http://support.microsoft.com/kb/117778/ja
これを参照のこと。

もっと詳しいのがあったと思うんですが?

PATIO 2009/10/09(金) 17:57:24
もし、引き渡したbrushがセレクト状態で削除できないと言うのであれば、
ポインタ変数を二つ用意しておいて以下のようにしてはいかがでしょう。

どちらが使用中なのかわかるステータス変数を用意します。
最初はどちらも使っていない状態にしておきます。
どちらも使っていない時は、ポインタ変数1でbrushを作成して
そのハンドルを返します。この時、ステータスをポインタ1使用中にします。

次に色を変更する時にポインタ変数2でbrushをnew。
そのbrushでCreateし、こちらのbrushのハンドルを
返します。この時、ステータスをポインタ2使用中にします。

以降は、色の変更が行われる度にステータスをチェックして使われていない方の
ポインタ変数をDeleteObjectしてCreate。ハンドル返却とステータスの更新を
行ないます。

利用されていない方のポインタ変数のNULLチェックで処理を分ければ、
同じ流れで二回目の変更とそれ以降は処理できるはず。

最終的に終了時に使用中のポインタ変数の方をDeleteObjectして
両方のポインタ変数をdeleteで開放する。

ポインタ変数を使わなくても実体にしておいてCreateしたかどうかフラグで
管理しても良いかもしれません。

次のブラシが設定された後なら前のブラシは非選択状態になっていると
思われるのでDeleteObjectできるのではないかと思います。

PATIO 2009/10/09(金) 18:01:27
基、

> 最終的に終了時に使用中のポインタ変数の方をDeleteObjectして
> 両方のポインタ変数をdeleteで開放する。

これ間違ってました。
終了時は両方のポインタに対してNULLチェックを行ない、
NULLでなければ、DeleteObjectした後にdeleteしてNULLクリアですね。
まあ、両方とも非選択状態ならいきなりdeleteしても開放されるとは
思いますけれど、この辺は気持ちの問題になるかなぁ。

vcshoshinsha 2009/10/09(金) 19:14:18
初心者ですみません、

>ポインタ変数1でbrushを作成して
とは

CBrush* m_hbr1; // エディット コントロールの背景色用メンバ変数

m_hbr1 = new CBrush(m_color);
のようでいいでしょうか?

また、この場合、DeleteでNULLとならないのですが、どのようにすればNULL判定できま
すか?

以下、ステータスで切り替えてやってみたソースです。
おかしいところが多々あると思いますが、ご指摘いただけると助かります。


HBRUSH w_hbr = CDialog::OnCtlColor(pDC, pWnd, nCtlColor);

switch (b_sts){
    case 0:
    case 1:
        if(m_hbr1 == NULL){
            m_hbr1 = new CBrush(m_color);
            SelectObject((HDC)pDC->GetSafeHdc(), (HBRUSH)m_hbr1);
            w_hbr = (HBRUSH)m_hbr1->GetSafeHandle();

            DeleteObject(m_hbr2);
            m_hbr2 = NULL;
            b_sts = 2;
        }
        break;
    case 2:
        if(m_hbr2 == NULL){
            m_hbr2 = new CBrush(m_color);
            SelectObject((HDC)pDC->GetSafeHdc(), (HBRUSH)m_hbr2->GetSafeHandle
());
            w_hbr = (HBRUSH)m_hbr2;

            DeleteObject(m_hbr1);
            m_hbr1 = NULL;
            b_sts = 1;
        }
        break;
    default:
        b_sts = 0;
        break;
}

return w_hbr;

ITO 2009/10/09(金) 19:23:02
ん?

「OnCtlColor」の処理で困っているのは、エディット コントロールだけですか?

リッチエディットは使ったことないですか?
使ったことがあるのならリッチエディットを薦めます。

vcshoshinsha 2009/10/09(金) 20:54:08
すみません。
STATICコントロールでした。
記述忘れてました。I/Oチェックのランプのような感じで使用したいと思っていました。

ITO 2009/10/10(土) 10:18:58
>STATICコントロールでした。
>記述忘れてました。I/Oチェックのランプのような感じで使用したいと思っていまし
た。
うーーん、
となると、ピクチャーコントロールを使って直接描くぐらいしか
今は思いつかないですね。
または、ボタンをその数だけ並べてすべてオーナードロー?
それもかなりしんどいですよね。

瀬戸っぷ [E-Mail] 2009/10/10(土) 14:08:49
OnCtlColor()で背景に設定するHBRUSHのSelectObject()必要ですか?

Win32SDKでプロパティシートで使ったものを組んだことがありますが…
WM_CTLCOLOR(実際はスタティックコントロールでしたのでWM_CTLCOLORSTATIC)の戻り値に
HBRUSHを返却しています。
次回WM_CTLCOLORが通知された際にDeleteObject()していますが、TRUEが返却されていま
すので成功している…かと。
# ちなみに、スタティックコントロールで文字も表示しているので…SetBkColor()も実行
してます。

ちなみに、作成したHBRUSHはグローバル変数で持たせています。
# プロパティシートを閉じるとき(ダイアログを閉じるとき)にDeleteObject()しています。

瀬戸っぷ [E-Mail] 2009/10/10(土) 14:44:00
え〜と…

>記述忘れてました。I/Oチェックのランプのような感じで使用したいと思っていました。

この状態が変わるのはどこ…でしょう?
状態が変わった際に、スタティックコントロールが再描画されるようにしていますか?
# CStatic::Invalidate()とかCStatic::RedrawWindow()とかCStatic::UpdateWindow()とか…
# CStatic::Invalidate()かCStatic::RedrawWindow()から背景の再描画フラグも必要で
しょう。

ダイアログにはそのスタティックコントロール用の背景ブラシとしてメンバ変数を作成、
初期化しておきます。

状態変更がダイアログ内のコントロールの操作(ボタン押すとか何か選択した等)ならば、
その時にCreateSolidBrush()でメンバ変数のCBrushを更新する。
ダイアログの操作ではなく、外部で状態変更されるのならばOnCtlColor()内で状態を参照
してCreateSolidBrush()で作成することになるかと。
# CBrush::DeeteObject()で削除してからCBrush::CreateSolidBrush()で作成。

OnCtlColor()では作成済みの背景ブラシを戻り値として渡す。
メンバ変数ならばダイアログクラスのデストラクタ実行時にCBrushのデストラクタで
DeleteObject()されるかと。

vcshoshinsha 2009/10/13(火) 08:16:50
回答いただいた方ありがとうございます。

>記述忘れてました。I/Oチェックのランプのような感じで使用したいと思っていまし
た。

のタイミングですが、100msタイマーでI/Oをチェックし、ONならON色を、OFFならOFF色
を返却する仕組みにしています。

    CBrush* m_hbr1;
    CBrush* m_hbr2;//メンバ変数

    m_hbr1 = NULL;
    m_hbr2 = NULL;


I/Oチェックでフラグを立てるのとInvalidate(FALSE)は、OnTimer内でやっています。
あとはOnCtlClrでフラグをみてブラシの切り替えをおこなっています。

上記のようにやると100k/1sぐらいでメモリが増えていきます。

vcshoshinsha 2009/10/13(火) 08:18:09
すみません、

    CBrush* m_hbr1;
    CBrush* m_hbr2;//メンバ変数

はダイアログクラスのヘッダー内、

    m_hbr1 = NULL;
    m_hbr2 = NULL;

はコンストラクタ内です。

PATIO 2009/10/13(火) 09:33:35
えーと、まずはDeleteObjectをやっている所で失敗しているかどうかを
ちゃんと確認する必要があると思いますよ。
今の書き込みの内容だとちゃんと裏づけが取られていないような感じです。
状況だけを見て判断してはいけないと思います。

DeleteObjectで成功しているのであれば、
他に原因があると言う事になると思います。

あと、ポインタ変数に関しては自分で管理し無いと駄目ですよ。

まず、クラスの初期化時にNULLクリアしておくのは基本として
deleteしたら、プログラマの責任でNULLクリアします。
これによって開放済みの領域の二重開放等を防ぎます。

ブラシのインスタンス作成に関しては、
new CBrush();して、その後でCreateSolidBrushしても良いし、
この辺は目的がきちんと達成できるならそれでよろしいかと。
私があげているのはあくまでもDeleteObjectしようとした時に
開放したいブラシオブジェクトがまだ選択状態になっていて
DeleteObjectに失敗している状況を想定しています。
そもそもDeleteObjectに失敗している事が確認できていないなら
方法論も何も無いので、失敗している事の確認がまず第一です。

vcshoshinsha 2009/10/13(火) 10:37:51
HBRUSH CDIODlg::OnCtlColor(CDC* pDC, CWnd* pWnd, UINT nCtlColor)
{
    HBRUSH w_hbr = CDialog::OnCtlColor(pDC, pWnd, nCtlColor);

    // TODO:  ここで DC の属性を変更してください。
    bool bRes;

    bRes = DeleteObject( w_hbr );
    w_hbr = (HBRUSH)::CreateSolidBrush(m_color);

    return w_hbr;
}

この段階で、
bRes = DeleteObject( w_hbr )自体trueが帰ってきていたので、成功していました。
ただ、メモリはゆっくり4kずつ増えていくのも確かです。

ですので、なにか失敗しているのでは、となったと思います。

PATIO 2009/10/13(火) 10:52:11
メモリが増えて行くと言うのは何で確認していますか?

メモリは延々と増え続けますか?

OSから見たメモリの開放とアプリから見たメモリ開放は
必ずしも等価では有りませんが、その点に関しては理解されていますか?
プログラム的には開放していてもアプリケーション的にリザーブして
いるケースがあると思います。
この場合は、ある程度以上はメモリの使用量は増えていかないはずなので
動作としては正常です。
特にタスクマネージャで確認していたりするとその部分に関しては
考慮せずに使用メモリを表示したと思うので宛にならない事があります。

ハンドルが開放されていなくてハンドルがリークしているのと
上記の現象は別になるのでその辺は分けて考える必要があると思います。

vcshoshinsha 2009/10/13(火) 11:01:05
タスクマネージャで確認していました。
2,3sで4k増えていっていました。

他に見る方法がわからなかったのでタスクマネージャで確認していましたが、
もう少し長時間でどうなるか確認してみようと思います。

subaru 2009/10/13(火) 11:07:59
>HBRUSH CDIODlg::OnCtlColor(CDC* pDC, CWnd* pWnd, UINT nCtlColor)
>{
>    HBRUSH w_hbr = CDialog::OnCtlColor(pDC, pWnd, nCtlColor);
>
>    // TODO:  ここで DC の属性を変更してください。
>    bool bRes;
>
>    bRes = DeleteObject( w_hbr );
>    w_hbr = (HBRUSH)::CreateSolidBrush(m_color);
>
>    return w_hbr;
>}

入力ミスなのかもしれないけど上記コードでは
w_hbrがローカル変数になっているのでリークしますよ。

vcshoshinsha 2009/10/13(火) 11:22:39
間違えていました。

HBRUSH CDIODlg::OnCtlColor(CDC* pDC, CWnd* pWnd, UINT nCtlColor)
{
    HBRUSH w_hbr = CDialog::OnCtlColor(pDC, pWnd, nCtlColor);

    // TODO:  ここで DC の属性を変更してください。
    bool bRes;

    bRes = DeleteObject( m_hbr );
    m_hbr = (HBRUSH)::CreateSolidBrush(m_color);
    w_hbr = m_hbr;

    return w_hbr;
}

メンバ変数にしたところ、明らかにメモリの増加がなくなりました。
一瞬増加したときもありましたが、先ほどのペースで考えると全く増加していないよう
に見えます。(タスクマネージャレベルですが)

解決したように見えます。
もう少し、長時間あとまで放置し、どうなるか確認してみます。

ありがとうございます。

ryo 2009/10/13(火) 12:08:08
おまけ

GDIオブジェクトの生成・解放の確認するなら
タスクマネージャのメモリの状況よりも、「GDIオブジェクト」をみたほうがいいよ

vcshoshinsha 2009/10/13(火) 13:43:09
>GDIオブジェクトの生成・解放の確認するなら
>タスクマネージャのメモリの状況よりも、「GDIオブジェクト」をみたほうがいいよ

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

vcshoshinsha 2009/10/14(水) 08:10:11
[[解決]]
今朝までのランニングテストではタスクマネジャー上ですが12kほど増えていましたが、
GDIオブジェクト数は増えてはいませんでした。

おそらく上記処理で問題なくいけたと思います。
いろいろご教授いただいた方、ありがとうございました。

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