Windows C#

Bug Report

C# Bug Report

About this Project

C++の経験が豊富なエンジニアがC#のプロパティを実装する際、オブジェクトの参照制御を誤解したことで、イベントが通知されない(通信の受信ができない)不具合が発生した。

Patterns that occurred

登場クラス

  • FromMain:GUIメイン画面。通信管理インスタンスを保持する。
  • FromEnv:GUI設定および通信テスト画面。
  • FromCommText:GUIテキスト通信画面。
  • CommWifi:Wifi通信管理。送信は非同期スレッド、受信・異常発生はイベントで通知する。

Workflow

  • 1. ユーザーはFromMainが起動
  • 2. FromMainがCommWifiを生成
  • 3. FromMainがFromEnvを生成
  • 4. FromMainはFromEnvのコンストラクタとしてCommWifiを渡す
  • 5. FromEnvはCommWifiの受信・異常イベントを購読
  • 6. ユーザーはFromEnvを用いて通信テストを行い、正常終了を確認
  • 7. ユーザーはFromEnvを閉じる
  • 8. ユーザーはFromCommTextを開く
  • 9. FromMainがFromCommTextを生成
  • 10. FromMainはFromCommTextのコンストラクタとしてCommWifiを渡す
  • 11. FromCommTextはCommWifiの受信・異常イベントを購読
  • 12. ユーザーはFromCommTextで通信開始
  • 13. 不具合発生: 送信は成功するが、イベント(受信通知)が一切来ない。

Root Cause: Incorrect Property Implementation

原因箇所となったコード(簡略化)は以下の通り。

public CommWifi CommWifi
{
    get => new CommWifi(); // アクセスのたびに新しいインスタンスを生成
}

Technical Background

実装者はC++の「実体返し(コピー)」に近いイメージで記述した可能性があるが、 C#においてこれは「アクセスのたびに独立した新しいインスタンスを生成して返す」動作となる。

  • `FromEnv` が受け取った `CommWifi` インスタンスと、`FromCommText` が受け取ったインスタンスは別物(参照が異なる)。
  • `FromMain` が実際に通信処理を行っている大元のインスタンスとも異なる。
  • その結果、イベントを購読しても、実際に動いているインスタンスとは別の場所を監視している状態になり、通知が届かない。

Conclusion

C#はポインタを明記しないが、`class` は常に参照(ポインタのようなもの)を扱っている。 C++エンジニアに対しては、**「classキーワードは常にスマートポインタを扱っているのと同義である」**と認識を合わせることが重要である。

Prevention and Review Guidelines

1. 正しいプロパティの実装(バッキングフィールドの利用)

意図せず新しいインスタンスを生成しないよう、初期化済みの同一インスタンスを返す。

private readonly CommWifi _commWifi = new CommWifi();
/// <summary>
/// 常に同一のインスタンス(参照)を返す
/// </summary>
public CommWifi CommWifi => _commWifi;

2. イベントの解除(メモリリーク防止)

画面を閉じた後も古い画面の購読が残っていると、予期せぬ挙動やメモリリークの原因となる。


// FromEnv 内での解除例
protected override void OnFormClosing(FormClosingEventArgs e)
{
    _commWifi.OnDataReceived -= HandleDataReceived; // 購読解除
    base.OnFormClosing(e);
}

3. レビューチェックリストへの追加

  • プロパティの副作用確認:get アクセサの中で new を行っていないか?(計算済みプロパティ以外での生成は避ける)
  • 同一性の確認:複数のクラスで共有するオブジェクトが、意図した通りに同じ参照(`object.ReferenceEquals`)を指しているか。
  • ライフサイクル管理: `+=` で登録したイベントは、オブジェクト破棄時(DisposeやClosing)に `-=` で解除されているか。

Notes

アプリの動作が「なんとなく重い」と感じたり、 特定画面を開くたびに挙動が不安定になる場合、 不用意なインスタンス生成(コピー)が行われていないか、 あるいはイベントの多重登録が発生していないかを疑うことが、 不具合の早期発見につながることもある。

Analysis

  • C#の自動実装プロパティ `public CommWifi CommWifi { get; } = new();` を使った場合の挙動について

    プロパティの実装が「常に new」している場合

    不具合のコード例では `get => new CommWifi();` となっていますが、もし以下のような実装になっていた場合、アクセスするたびに新しいインスタンスが生成される。

    アクセスするたびに新しいインスタンスが作られる(バグの原因になりやすい例)

    
    public CommWifi CommWifi => new CommWifi();
                        

    これでは、FromEnv と FromCommText でそれぞれ別のインスタンスを操作していることになります。

    コード改善案

    不具合の再発を防ぎ、C#らしい安全な設計にする。

    
    private readonly CommWifi _commWifi = new CommWifi();
    
    // 常に同じインスタンス(参照)を返す
    public CommWifi CommWifi => _commWifi;
                        
  • イベントハンドラの解除を漏らさないための、IDisposableパターンやWeak Eventパターンについて

    イベントの解除(メモリリークと予期せぬ動作の防止)

    フローにおいて、FromEnv を閉じた後に FromCommText を開いています。C#のイベントは、解除を忘れるとインスタンスがGC(ガベージコレクション)されず、メモリリークや予期せぬ挙動の原因になります。

    
    // FromEnv 内での適切な解除
    protected override void OnFormClosing(FormClosingEventArgs e)
    {
        // イベント購読を解除しないと、FormEnvがメモリに残り続ける可能性がある
        _commWifi.OnDataReceived -= HandleDataReceived;
        base.OnFormClosing(e);
    }
                        
  • コードレビューのチェックポイント(追加案)

    「コピーの理解」に加え、以下の観点をチェックリストに加えることを推奨。

    プロパティでの副作用:

    get アクセサの中で new を行っていないか?(計算済みプロパティ以外でのインスタンス生成は避ける)

    イベントのライフサイクル:

    += で登録したイベントは、オブジェクト破棄時(Dispose や Closing)に -= で解除されているか?

    同一性の確認:

    デバッグ時に object.ReferenceEquals(obj1, obj2) を使用して、意図した通りに同じインスタンスを指しているか確認する習慣があるか?

結論

C#はポインタを隠蔽していますが、内部的には「参照(ポインタのようなもの)」を扱っています。C++エンジニアに対しては、 「classキーワードは常にポインタ(スマートポインタ)を扱っているのと同義である」と説明すると、理解がスムーズに進むことが多い。

今回のケースは、言語仕様の差異が設計意図を歪めてしまった好例であり、ナレッジ共有は非常に価値が高い。

Video