SetTimer Callback is Never Called

My C++ application has a windowless timer to periodically cleanup latent communications data that was never (and will never be) fully processed. The problem is that the callback function is never called. My class constructor executes the following code, just before it returns:

    if ( (this->m_hMsgsThread = ::CreateThread(
        NULL,                           // no security attributes
        0,                              // use default initial stack size
        reinterpret_cast<LPTHREAD_START_ROUTINE>(MessagesThreadFn), // function to execute in new thread
        this,                           // thread parameters
        0,                              // use default creation settings
        NULL                            // thread ID is not needed
        )) == NULL )
    {
        dwError = ::GetLastError();
        TRACE(_T("%s : Failed to create thread.\r\n\tError: %d\r\n\tFile: %s\r\n\tLine: %d\r\n"), _T(__FUNCTION__), dwError, _T(__FILE__), __LINE__);
        continue;
    }

    if ( (s_pCleanupTimerId = ::SetTimer( NULL, 0, MSGS_CLEANUP_PERIOD, CleanupTimerProc )) == NULL )
    {
        dwError = ::GetLastError();
        TRACE(_T("%s : Failed to create timer.\r\n\tError: %d\r\n\tFile: %s\r\n\tLine: %d\r\n"), _T(__FUNCTION__), dwError, _T(__FILE__), __LINE__);
        continue;
    }

This is my definition for CleanupTimerProc:

static void CALLBACK CleanupTimerProc( HWND hwnd, UINT uMsg, UINT_PTR idEvent, DWORD dwTime )
{
    CMsgDesc * pobjMsgDesc = NULL;
    DWORD dwError = ERROR_SUCCESS;
    DWORD dwItem;
    DWORD dwList;
    DWORD dwListItems;
    DWORD dwThen, dwNow;
    const DWORD cMAX_LISTS = MSGS_MAX_EVENTS;

    do
    {
        // Kill off the old timer.
        TRACE(_T("%s : Killing cleanup timer.\r\n"), _T(__FUNCTION__));
        ASSERT(s_pCleanupTimerId == idEvent);
        ::KillTimer( hwnd, idEvent );

        // Start a new timer using the same ID.
        TRACE(_T("%s : Restarting cleanup timer.\r\n"), _T(__FUNCTION__));
        if ( (s_pCleanupTimerId = ::SetTimer( NULL, 0, MSGS_CLEANUP_PERIOD, CleanupTimerProc )) == NULL )
        {
            dwError = ::GetLastError();
            TRACE(_T("%s : Failed to create timer.\r\n\tError: %d\r\n\tFile: %s\r\n\tLine: %d\r\n"), _T(__FUNCTION__), dwError, _T(__FILE__), __LINE__);
            continue;
        }

        // Get the current time.
        dwNow = ::GetTickCount();

        // Search through the message descriptor lists
        // looking for descriptors that haven't been touched in a while.
        TRACE(_T("%s : Deleting old message descriptors.\r\n"), _T(__FUNCTION__));
        ASSERT(s_pInterface != NULL);
        ASSERT(s_pInterface->pobjMessages != NULL);
        ::Ent开发者_开发知识库erCriticalSection( &s_pInterface->pobjMessages->m_csMsgDescriptors );
        for ( dwList = 0; dwList < cMAX_LISTS; dwList++ )
        {
            dwListItems = s_pInterface->pobjMessages->m_pobjMsgDescriptors[dwList]->GetItemCount();

            for ( dwItem = 0; dwItem < dwListItems; dwItem++ )
            {
                if ( (pobjMsgDesc = (CMsgDesc *)s_pInterface->pobjMessages->m_pobjMsgDescriptors[dwList]->Peek( NULL, dwItem )) == NULL )
                {
                    dwError = ::GetLastError();
                    TRACE(_T("%s : Failed to peek item from list.\r\n\tError: %d\r\n\tFile: %s\r\n\tLine: %d\r\n"), _T(__FUNCTION__), dwError, _T(__FILE__), __LINE__);
                    continue;
                }

                // Get the last touched time from the descriptor.
                dwThen = pobjMsgDesc->m_dwLastTouched;

                // If the specified time has elapsed, delete the descriptor.
                if ( (dwNow - dwThen) > MSGS_DESC_SHELFLIFE )
                {
                    if ( s_pInterface->pobjMessages->m_pobjMsgDescriptors[dwList]->Remove( NULL, dwItem ) == NULL )
                    {
                        dwError = ::GetLastError();
                        TRACE(_T("%s : Failed to remove item from list.\r\n\tError: %d\r\n\tFile: %s\r\n\tLine: %d\r\n"), _T(__FUNCTION__), dwError, _T(__FILE__), __LINE__);
                        continue;
                    }

                    delete pobjMsgDesc;
                    TRACE(_T("%s : Deleted old message descriptor.\r\n"), _T(__FUNCTION__));
                }
            }
        }
        ::LeaveCriticalSection( &s_pInterface->pobjMessages->m_csMsgDescriptors );
    }
    while ( 0 );
}

Any thoughts as to why this function is not getting called? Do I need to create the timer from within the thread?

I may be remembering this incorrectly ....

However a timer still requires a windows message pump. If you want that timer to fire in a given thread then that thread need to pump messages or it will never get called.

Equally it seems to me that you are creating a timer callback that enters an infinite loop. The function needs to exit or the next timer can never get called.

Use CreateWaitableTimer() and SetWaitableTimer() instead of SetTimer(). Waitable timers work with the WaitFor...() family of functions, like MsgWaitForMultipleObjects(), eg:

HANDLE s_pCleanupTimer;

s_pCleanupTimer = CreateWaitableTimer(NULL, FALSE, NULL);
if( !s_pCleanupTimer )
{
    dwError = ::GetLastError();
    TRACE(_T("%s : Failed to create timer.\r\n\tError: %d\r\n\tFile: %s\r\n\tLine: %d\r\n"), _T(__FUNCTION__), dwError, _T(__FILE__), __LINE__);
    continue;
}

LARGE_INTEGER DueTime;
DueTime.LowPart = -(MSGS_CLEANUP_PERIOD * 10000);
DueTime.HighPart = 0;
if( !SetWaitableTimer(s_pCleanupTimer, &DueTime, MSGS_CLEANUP_PERIOD, NULL, NULL, FALSE) )
{
    dwError = ::GetLastError();
    TRACE(_T("%s : Failed to start timer.\r\n\tError: %d\r\n\tFile: %s\r\n\tLine: %d\r\n"), _T(__FUNCTION__), dwError, _T(__FILE__), __LINE__);
    CloseHandle(s_pCleanupTimer);
    s_pCleanupTimer = NULL;
    continue;
}

Then in your message loop (or any other kind of status poll):

do
{
    DWORD dwRet = MsgWaitForMultipleObjects(1, &hTimer, FALSE, INFINITE, QS_ALLINPUT);
    if( dwRet == WAIT_FAILED )
        break;

    if( dwRet == WAIT_OBJECT_0 ) // timer elapsed
        CleanupTimerProc();

    else if( dwRet == (WAIT_OBJECT_0+1) ) // pending message
        ProcessPendingMessages();
}
while( true );

In your function I see that you're killing off the old timer and starting the new one and it looks rather redundant. I believe Goz is right when he says that you're creating an infinite loop.

I wrote a short program using the same non-window-based unidentified timers that you're using and it works fine for me.

#include <windows.h>

VOID CALLBACK TimerProc(HWND hWnd, UINT uMessage, UINT_PTR uEventId, DWORD dwTime)
{
  UNREFERENCED_PARAMETER(hWnd);
  UNREFERENCED_PARAMETER(uMessage);
  UNREFERENCED_PARAMETER(uEventId);
  UNREFERENCED_PARAMETER(dwTime);

  MessageBox(NULL, "lol", "lol", MB_OK);
}

LRESULT CALLBACK WndProc(HWND hWnd, UINT uMessage, WPARAM wParam, LPARAM lParam)
{
  switch (uMessage)
  {
  case WM_CREATE:
    SetTimer(NULL, 0, 1000, TimerProc);
    break;

  case WM_CLOSE:
    DestroyWindow(hWnd);
    break;

  case WM_DESTROY:
    PostQuitMessage(0);
    break;
  }
  return DefWindowProc(hWnd, uMessage, wParam, lParam);
}

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpszCommandLine, int nShowCommand)
{
  UNREFERENCED_PARAMETER(hPrevInstance);
  UNREFERENCED_PARAMETER(lpszCommandLine);
  UNREFERENCED_PARAMETER(nShowCommand);

  WNDCLASSEX wce;

  ZeroMemory(&wce, sizeof(wce));
  wce.cbSize = sizeof(wce);
  wce.hInstance = hInstance;
  wce.lpfnWndProc = WndProc;
  wce.lpszClassName = "test";

  if (RegisterClassEx(&wce) > 0)
  {
    if (CreateWindow("test", "test", 0, 0, 0, 0, 0, HWND_MESSAGE, NULL, hInstance, NULL) != NULL)
    {
      MSG msg;

      while (GetMessage(&msg, NULL, 0, 0) > 0)
      {
        TranslateMessage(&msg);
        DispatchMessage(&msg);
      }
      return static_cast<int>(msg.wParam);
    }
  }
  return EXIT_FAILURE;
}