28

マルチスレッドアプリケーションでクラッシュをデバッグしているときに、最終的に次のステートメントで問題を特定しました。

CSingleLock(&m_criticalSection, TRUE);

CSingleLock クラスの名前のないオブジェクトを作成しているため、このステートメントの直後にクリティカル セクション オブジェクトのロックが解除されることに注意してください。これは明らかに、コーダーが望んでいたことではありません。このエラーは単純な入力ミスが原因でした。私の質問は、コンパイル時にクラスの一時オブジェクトが作成されるのを防ぐことができる方法があるかどうかです。つまり、上記のタイプのコードはコンパイラエラーを生成するはずです。一般に、クラスがある種のリソース取得を試みるときはいつでも、そのクラスの一時オブジェクトは許可されるべきではないと思います。それを強制する方法はありますか？

4

8 に答える 8

14

編集： j_random_hackerが指摘しているように、ロックを解除するために、ユーザーに名前付きオブジェクトの宣言を強制することができます。

ただし、クラスで一時的な作成が何らかの理由で禁止されている場合でも、ユーザーは同様の間違いを犯す可能性があります。

// take out a lock:
if (m_multiThreaded)
{
    CSingleLock c(&m_criticalSection, TRUE);
}

// do other stuff, assuming lock is held

最終的に、ユーザーは自分が書いたコード行の影響を理解する必要があります。この場合、オブジェクトを作成していることを知っている必要があり、オブジェクトがどのくらい続くかを知っている必要があります。

もう1つの考えられる間違い：

 CSingleLock *c = new CSingleLock(&m_criticalSection, TRUE);

 // do other stuff, don't call delete on c...

「クラスのユーザーがヒープに割り当てるのを防ぐ方法はありますか？」と尋ねるのはどれですか。答えは同じだろう。

C ++ 0xでは、ラムダを使用して、これをすべて行う別の方法があります。関数を定義します。

template <class TLock, class TLockedOperation>
void WithLock(TLock *lock, const TLockedOperation &op)
{
    CSingleLock c(lock, TRUE);
    op();
}

この関数は、CSingleLockの正しい使用法をキャプチャします。次に、ユーザーにこれを実行させます。

WithLock(&m_criticalSection, 
[&] {
        // do stuff, lock is held in this context.
    });

これは、ユーザーが台無しにするのがはるかに困難です。構文は最初は奇妙に見えますが、[＆]の後にコードブロックが続くということは、「引数をとらない関数を定義することを意味します。名前で何かを参照し、それが外部の何かの名前である場合（たとえば、含まれているローカル変数）関数）非定数参照でアクセスさせて、変更できるようにします。）

于 2009-05-27T10:10:10.070 に答える
3

いいえ、これを行う方法はありません。そうすることで、名前のない一時的なものの作成に大きく依存するほとんどすべてのC++コードが破損します。特定のクラスに対する唯一の解決策は、コンストラクターをプライベートにしてから、常に何らかのファクトリを介してコンストラクターを構築することです。しかし、私は治療法が病気よりも悪いと思います！

于 2009-05-27T10:00:24.473 に答える
2

私はそうは思わない。

バグでわかったように、それは賢明なことではありませんが、ステートメントについて「違法」なことは何もありません。コンパイラは、メソッドからの戻り値が「バイタル」かどうかを知る方法がありません。

于 2009-05-27T09:49:17.257 に答える
2

コンパイラは一時オブジェクトの作成を禁止すべきではありません、IMHO。

ベクトルを縮小するような特殊なケースでは、一時オブジェクトを作成する必要があります。

std::vector<T>(v).swap(v);

少し難しいですが、コード レビューと単体テストでこれらの問題を検出する必要があります。

それ以外の場合は、貧しい人の解決策が 1 つあります。

CSingleLock aLock(&m_criticalSection); //Don't use the second parameter whose default is FALSE

aLock.Lock();  //an explicit lock should take care of your problem
于 2009-05-27T09:57:23.950 に答える
1

以下はどうですか？プリプロセッサをわずかに悪用しますが、それを含める必要があると思うほど賢いです:

class CSingleLock
{
    ...
};
#define CSingleLock class CSingleLock

以下は有効なC++であるため、一時的な結果に名前を付けるのを忘れるとエラーになります。

class CSingleLock lock(&m_criticalSection, true); // Compiles just fine!

名前を省略した同じコードは、次のようにはなりません。

class CSingleLock(&m_criticalSection, true); // <-- ERROR!
于 2014-08-28T22:53:47.100 に答える
0

この 5 年間、誰も最も単純な解決策を思い付いていないことがわかりました。

#define LOCK(x) CSingleLock lock(&x, TRUE);
...
void f() {
   LOCK(m_criticalSection);

そして今、このマクロはロックを作成するためにのみ使用してください。もう一時的なものを作成する機会はありません! これには、不適切な再帰ロックの検出、ロックのファイルと行の記録など、デバッグ ビルドであらゆる種類のチェックを実行するためにマクロを簡単に拡張できるという追加の利点があります。

于 2014-08-01T10:47:32.573 に答える
0

古い質問ですが、追加する点がいくつかあります。

クラスと同じ名前のマクロ関数を定義することで、誰かが変数名を忘れたときに役立つメッセージで静的アサーションをトリガーできます。ここに住む

class CSingleLock {
 public:
  CSingleLock (std::mutex*, bool) { }
};

// must come after class definition
#define CSingleLock(...) static_assert(false, \
    "Temporary CSingleLock objects are forbidden, did you forget a variable name?")

変数名がある場合、マクロは一致しません。ただし、これは一様な初期化の場合には役に立ちません。あなたはキャッチできませんCSingleLock{&m, true}。PfhorSlayer の回答は均一な初期化で機能するため、より混乱するエラー メッセージを犠牲にして、より安全に使用できます。私はまだ私の解決策よりもその解決策をお勧めします。残念ながら、型が名前空間にある場合、これらのマクロ ソリューションはすべて失敗します。

別の解決策は、次を使用してコンパイラ警告を生成することです[[nodiscard]]

class CSingleLock {
 public:
  [[nodiscard]] CSingleLock (std::mutex*, bool) { }
};

一時的なclangを作成すると、次のように警告されます:

warning: ignoring temporary created by a constructor declared with 'nodiscard' attribute [-Wunused-value]
  CSingleLock(&m, true);
  ^~~~~~~~~~~~~~~~~~~~~

ただし、GCC 9.2 では[[nodiscard]]on コンストラクターに問題があるようです。結果を使用しない場合でも、追加の警告が表示されます。この問題は、執筆時点で wandbox の gcc-10 20191217 である head で修正されています。

于 2019-12-22T20:05:22.863 に答える