116039 2013-05-13 09:15:03 -0700 [WK2][Win] Fix ASSERT(DeleteTimerQueueTimer...) 2013-05-14 05:53:43 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jturcotte webkit-unassigned hausmann jturcotte oldest_to_newest 888309 0 jturcotte 2013-05-13 09:15:03 -0700 [WK2][Win] Fix ASSERT(DeleteTimerQueueTimer...) 888310 1 201574 jturcotte 2013-05-13 09:16:41 -0700 Created attachment 201574 Patch 888324 2 201574 darin 2013-05-13 09:46:49 -0700 Comment on attachment 201574 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? 888340 3 jturcotte 2013-05-13 10:15:44 -0700 (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. 888696 4 201574 benjamin 2013-05-13 22:15:34 -0700 Comment on attachment 201574 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 :) 888803 5 jturcotte 2013-05-14 05:53:43 -0700 Committed r150068: <http://trac.webkit.org/changeset/150068> 201574 2013-05-13 09:16:41 -0700 2013-05-13 22:15:33 -0700 Patch DeleteTimerQueueTimerAssert.patch text/plain 1586 jturcotte ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCA0NDczZmFmLi4wOTUyNjkzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYg QEAKKzIwMTMtMDUtMTMgIEpvY2VseW4gVHVyY290dGUgIDxqb2NlbHluLnR1cmNvdHRlQGRpZ2lh LmNvbT4KKworICAgICAgICBbV0syXVtXaW5dIEZpeCBBU1NFUlQoRGVsZXRlVGltZXJRdWV1ZVRp bWVyLi4uKQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTE2MDM5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg QWNjb3JkaW5nIHRvIHRoZSBkb2N1bWVudGF0aW9uLCBEZWxldGVUaW1lclF1ZXVlVGltZXIgY2Fu IGJlIGV4cGVjdGVkIHRvIHJldHVybgorICAgICAgICBmYWxzZSB3aXRoIGFuIEVSUk9SX0lPX1BF TkRJTkcgZXJyb3Igd2hlbiBjYWxsZWQgZnJvbSB0aGUgdGltZXIncyBjYWxsYmFjay4KKworICAg ICAgICAqIFBsYXRmb3JtL3dpbi9Xb3JrUXVldWVXaW4uY3BwOgorICAgICAgICAoV29ya1F1ZXVl Ojp0aW1lckNhbGxiYWNrKToKKwogMjAxMy0wNS0xMyAgWmFsYW4gQnVqdGFzICA8emFsYW5AYXBw bGUuY29tPgogCiAgICAgICAgIFdlYlByb2Nlc3MgY29uc3VtaW5nIHZlcnkgaGlnaCBDUFUgb24g bGlua2VkaW4uY29tCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9QbGF0Zm9ybS93aW4vV29y a1F1ZXVlV2luLmNwcCBiL1NvdXJjZS9XZWJLaXQyL1BsYXRmb3JtL3dpbi9Xb3JrUXVldWVXaW4u Y3BwCmluZGV4IGQ2ZThlNGUuLmYyZTFiMGYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1Bs YXRmb3JtL3dpbi9Xb3JrUXVldWVXaW4uY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQyL1BsYXRmb3Jt L3dpbi9Xb3JrUXVldWVXaW4uY3BwCkBAIC0yMjgsNyArMjI4LDcgQEAgdm9pZCBXb3JrUXVldWU6 OnRpbWVyQ2FsbGJhY2sodm9pZCogY29udGV4dCwgQk9PTEVBTiB0aW1lck9yV2FpdEZpcmVkKQog ICAgIEFTU0VSVCh0aW1lckNvbnRleHQtPnRpbWVyKTsKICAgICBBU1NFUlQodGltZXJDb250ZXh0 LT5xdWV1ZS0+bV90aW1lclF1ZXVlKTsKICAgICBpZiAoITo6RGVsZXRlVGltZXJRdWV1ZVRpbWVy KHRpbWVyQ29udGV4dC0+cXVldWUtPm1fdGltZXJRdWV1ZSwgdGltZXJDb250ZXh0LT50aW1lciwg MCkpCi0gICAgICAgIEFTU0VSVF9XSVRIX01FU1NBR0UoZmFsc2UsICI6OkRlbGV0ZVRpbWVyUXVl dWVUaW1lciBmYWlsZWQgd2l0aCBlcnJvciAlbHUiLCA6OkdldExhc3RFcnJvcigpKTsKKyAgICAg ICAgQVNTRVJUX1dJVEhfTUVTU0FHRSg6OkdldExhc3RFcnJvcigpID09IEVSUk9SX0lPX1BFTkRJ TkcsICI6OkRlbGV0ZVRpbWVyUXVldWVUaW1lciBmYWlsZWQgd2l0aCBlcnJvciAlbHUiLCA6Okdl dExhc3RFcnJvcigpKTsKIH0KIAogdm9pZCBXb3JrUXVldWU6OmRpc3BhdGNoQWZ0ZXJEZWxheShj b25zdCBGdW5jdGlvbjx2b2lkKCk+JiBmdW5jdGlvbiwgZG91YmxlIGRlbGF5KQo=