Bug 116039 – [WK2][Win] Fix ASSERT(DeleteTimerQueueTimer...)

• Component: WebKit2
• Status: RESOLVED FIXED

[Jocelyn Turcotte]

[WK2][Win] Fix ASSERT(DeleteTimerQueueTimer...)

[Jocelyn Turcotte]

Created attachment 201574 [details]
Patch

[Darin Adler]

Comment on attachment 201574 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=201574&action=review

> Source/WebKit2/Platform/win/WorkQueueWin.cpp:231
>      if (!::DeleteTimerQueueTimer(timerContext->queue->m_timerQueue, timerContext->timer, 0))
> -        ASSERT_WITH_MESSAGE(false, "::DeleteTimerQueueTimer failed with error %lu", ::GetLastError());
> +        ASSERT_WITH_MESSAGE(::GetLastError() == ERROR_IO_PENDING, "::DeleteTimerQueueTimer failed with error %lu", ::GetLastError());

OK, but that still doesn’t answer the question of whether the error is acceptable. Did the deletion fail in this case? Will the timer be left around?

[Jocelyn Turcotte]

(In reply to comment #2)
> OK, but that still doesn’t answer the question of whether the error is acceptable. Did the deletion fail in this case? Will the timer be left around?

Quoting the documentation:
"If there are outstanding callback functions and CompletionEvent is NULL, the function will fail and set the error code to ERROR_IO_PENDING. This indicates that there are outstanding callback functions. Those callbacks either will execute or are in the middle of executing. The timer is cleaned up when the callback function is finished executing."

So if I read it right it should be properly cleaned in every case.
A bit of googling also reveals other projects not considering ERROR_IO_PENDING as an unexpected error in this situation.

[Benjamin Poulain]

Comment on attachment 201574 [details]
Patch

The patch looks good given your last comment. But it looks like your comment should be in the code in this case. Otherwise anyone but you will find this assertion mysterious :)

[Jocelyn Turcotte]

Committed r150068: <http://trac.webkit.org/changeset/150068>