3
\$\begingroup\$

Is this reliable, or have I missed something?

I want a drop-in replacement for WaitForSingleObject (sans the return type, I like HRESULT much better than any other error codes).

This code works in a plugin DLL for third party closed-source software. I don’t have source code of that app, i.e. if you ask “what else might happen on the same thread while it waits?” the answer is “anything at all”.

constexpr HRESULT E_TIMEOUT = HRESULT_FROM_WIN32( ERROR_TIMEOUT );
constexpr int MSGF_SLEEPMSG = 0x5300;
inline HRESULT getLastHr() { return HRESULT_FROM_WIN32( GetLastError() ); }

// Same as WaitForSingleObject but processes windows messages while it waits.
HRESULT msgWaitForSingleObject( HANDLE h, DWORD ms )
{
    // https://blogs.msdn.microsoft.com/oldnewthing/20060126-00/?p=32513/
    while( true )
    {
        const DWORD started = GetTickCount();
        const DWORD res = MsgWaitForMultipleObjectsEx( 1, &h, ms, QS_ALLINPUT, MWMO_INPUTAVAILABLE );
        if( WAIT_OBJECT_0 == res )
            return S_OK;
        if( WAIT_OBJECT_0 + 1 == res )
        {
            MSG msg;
            while( PeekMessage( &msg, NULL, 0, 0, PM_REMOVE ) )
            {
                if( msg.message == WM_QUIT )
                {
                    PostQuitMessage( (int)msg.wParam );
                    return S_FALSE; // Abandoned due to WM_QUIT
                }
                if( !CallMsgFilter( &msg, MSGF_SLEEPMSG ) )
                {
                    TranslateMessage( &msg );
                    DispatchMessage( &msg );
                }
            }
            const DWORD elapsed = GetTickCount() - started; //< This works OK even when DWORD overflows: https://codereview.stackexchange.com/a/129670/46194
            if( elapsed > ms )
                return E_TIMEOUT;
            ms -= elapsed;
            continue;
        }
        if( WAIT_TIMEOUT == res )
            return E_TIMEOUT;
        HRESULT hr = getLastHr();
        if( FAILED( hr ) )
            return hr;
        return E_UNEXPECTED;    // Unasked IO_COMPLETION or ABANDONED?
    }
}
\$\endgroup\$
  • \$\begingroup\$ From puristic code-review point of view: why parameters are mutable variables in this function? ms -= elapsed. I myself would make such int parameter const and copy it to non-const variable. The optimization compiler would completely minimize the overhead but it helps in debug mode so that we see the parameter without going up the stack. \$\endgroup\$ – Alexander V Dec 29 '18 at 17:51
  • \$\begingroup\$ @AlexanderV With 2 variables, you can accidentally use the wrong one of them, introducing a bug. Timeouts happen rarely, such bug will likely go unnoticed until it’s too late and the code’s deployed in production. When there’s only a single variable for remaining time, you can’t fail like that. \$\endgroup\$ – Soonts Dec 29 '18 at 18:07
  • \$\begingroup\$ Right, use const modifier for the parameter. \$\endgroup\$ – Alexander V Dec 29 '18 at 22:57
  • \$\begingroup\$ @AlexanderV Doesn’t help. MsgWaitForMultipleObjectsEx will happily accept const input variable instead of the non-const. \$\endgroup\$ – Soonts Dec 30 '18 at 1:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.