Bug 116039 - [WK2][Win] Fix ASSERT(DeleteTimerQueueTimer...)
Summary: [WK2][Win] Fix ASSERT(DeleteTimerQueueTimer...)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-13 09:15 PDT by Jocelyn Turcotte
Modified: 2013-05-14 05:53 PDT (History)
2 users (show)

See Also:


Attachments
Patch (1.55 KB, patch)
2013-05-13 09:16 PDT, Jocelyn Turcotte
benjamin: review+
benjamin: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jocelyn Turcotte 2013-05-13 09:15:03 PDT
[WK2][Win] Fix ASSERT(DeleteTimerQueueTimer...)
Comment 1 Jocelyn Turcotte 2013-05-13 09:16:41 PDT
Created attachment 201574 [details]
Patch
Comment 2 Darin Adler 2013-05-13 09:46:49 PDT
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?
Comment 3 Jocelyn Turcotte 2013-05-13 10:15:44 PDT
(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.
Comment 4 Benjamin Poulain 2013-05-13 22:15:34 PDT
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 :)
Comment 5 Jocelyn Turcotte 2013-05-14 05:53:43 PDT
Committed r150068: <http://trac.webkit.org/changeset/150068>