ぼやきごと/2016-03-04/GetLastError 関数に頼るのは危険 の変更点

#blog2navi()
*GetLastError 関数に頼るのは危険 [#t912c839]

下記のような関数があるとします。

#code(c){{
#include <string>
#include <Windows.h>

// IDに対応するミューテクスハンドルを返す。
// 既に作成済みの場合は nullptr を返す。
::HANDLE createUniqueMutex(int id)
{
    ::HANDLE mutex = ::CreateMutexW(nullptr, FALSE, std::to_wstring(id).data());

    if (mutex != nullptr && ::GetLastError() == ERROR_ALREADY_EXISTS)
    {
        // 既に作成済みなので nullptr を返す
        ::CloseHandle(mutex);
        mutex = nullptr;
    }

    return mutex;
}
}}

処理内容は下記の通りです。

+引数に渡した数値を @code{std::to_wstring}; 関数で文字列に変換する。
+1. の文字列を名前として @code{CreateMutexW}; 関数を呼び出し、ミューテクスを作成。
+@code{GetLastError}; 関数の戻り値が @code{ERROR_ALREADY_EXISTS}; ならば既に作成済みのミューテクスなので解放して @code{nullptr}; を返す。

特に問題はなさそうに見えますが、ある特定の状況だとこの関数は意図通りに動きません。

その状況とは、グローバルの @code{delete}; 演算子がオーバロードされ、その中で @code{GetLastError}; 関数の内部値が上書きされた場合です。

#code(c){{
#include <cstdlib>
#include <Windows.h>

void* operator new(std::size_t size)
{
    return std::malloc(size);
}

void operator delete(void* ptr)
{
    // GetLastError の内部値を上書きするような処理
    ::SetLastError(0);

    std::free(ptr);
}
}}

上記の例では直接 @code{SetLastError}; 関数を呼んでいますが、実際には何らかの Win32 API が呼ばれているものと思ってください。

先のコードの、 @code{CreateMutexW}; 関数を呼んでいる文は次の通りです。

#code(c,nomenu,8-){{
    ::HANDLE mutex = ::CreateMutexW(nullptr, FALSE, std::to_wstring(id).data());
}}

@code{std::to_wstring(id)}; で作成した @code{std::wstring}; は特に変数等に束縛されていないため、 @code{CreateMutexW}; 関数から処理が戻った時点でデストラクタが呼び出されます。~
@code{std::to_wstring(id)}; で作成した @code{std::wstring}; は特に変数等に束縛されていないため、この文の処理が完了した時点でデストラクタが呼び出されます。~
@code{std::wstring}; のデストラクタは、内部に保持しているバッファを @code{delete}; 演算子によって解放します。~
よって、 @code{GetLastError}; 関数を呼び出すよりも前に @code{operator delete}; が呼び出され、内部値が上書きされてしまいます。

@code{GetLastError}; 関数を利用するコードと @code{delete}; 演算子のオーバロードをどちらも自身で書いたならば気付けるかもしれませんが、下記のような場合は非常に気付きにくいです。

-外部ライブラリに上述のような @code{GetLastError}; 関数を利用するコードが入っている場合。
-外部ライブラリやフレームワークによってグローバルの @code{delete}; 演算子がオーバロードされている場合。

グローバルの @code{new};, @code{delete}; 演算子のオーバロードはリンクしているライブラリにまで波及するのがとても厄介です。

両者の組み合わせが一番最悪で、どちらかのソースコードを修正可能でなければどうしようもなくなってしまいます。~
私は、自身で作ったライブラリと Unreal Engine 4 （ゲームエンジン）の組み合わせでこの問題にハマり、原因判明までに丸1日を要しました…。

個人的には「グローバルの @code{new};, @code{delete}; 演算子をオーバロードするのはやめろ」と言いたいのですが、外製のフレームワークにやられていたらどうしようもないですし、 @code{GetLastError}; 関数の仕組みが危険なのもまた事実なので、注意してコーディングするしかないですかね。

なお、今回のコード例であれば、 @code{std::to_wstring(id)}; の結果を一旦変数に受け取ることで問題を回避できます。~
より安全にするなら、最初に @code{OpenMutexW}; 関数によって既存のミューテクスが存在するか確認するようにすれば、 @code{GetLastError}; 関数を使わずとも同等の処理が可能です。

RIGHT:Category: &#x5b;[[C++>ぼやきごと/カテゴリ/C++]]&#x5d;&#x5b;[[Visual Studio>ぼやきごと/カテゴリ/Visual Studio]]&#x5d;&#x5b;[[プログラミング>ぼやきごと/カテゴリ/プログラミング]]&#x5d; - 2016-03-04 00:29:24
----
RIGHT:&blog2trackback();
#comment(above)
#blog2navi()