Bug 176064 – [Win] Crash under WorkQueue::performWorkOnRegisteredWorkThread in layout tests.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Per Arne Vollan]

FAULTING_IP: 
WTF!WTF::WorkQueue::performWorkOnRegisteredWorkThread+70 [c:\cygwin\home\buildbot\slave\win-release\build\source\wtf\wtf\win\workqueuewin.cpp @ 60]
71d0b1a0 8b0e            mov     ecx,dword ptr [esi]

EXCEPTION_RECORD:  ffffffffffffffff -- (.exr 0xffffffffffffffff)
.exr 0xffffffffffffffff
ExceptionAddress: 0000000071d0b1a0 (WTF!std::unique_ptr<WTF::Function<void __cdecl(void)>::CallableWrapperBase,std::default_delete<WTF::Function<void __cdecl(void)>::CallableWrapperBase> >::get)
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 0000000000000000
   Parameter[1]: 0000000000000000
Attempt to read from address 0000000000000000

CONTEXT:  0000000000000000 -- (.cxr 0x0;r)
.cxr 0x0;r
eax=090dd7d8 ebx=24375f60 ecx=08ef1484 edx=00289208 esi=00000000 edi=08ef147c
eip=71d0b1a0 esp=0928f798 ebp=0928f7b4 iopl=0         nv up ei pl nz na po nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010202
WTF!std::unique_ptr<WTF::Function<void __cdecl(void)>::CallableWrapperBase,std::default_delete<WTF::Function<void __cdecl(void)>::CallableWrapperBase> >::get [inlined in WTF!WTF::WorkQueue::performWorkOnRegisteredWorkThread+0x70]:
71d0b1a0 8b0e            mov     ecx,dword ptr [esi]  ds:002b:00000000=????????
.cxr

STACK_TEXT:  
0928f7b4 71d0b0f0 08ef1578 0928f834 77da7edf WTF!WTF::WorkQueue::performWorkOnRegisteredWorkThread+0x70
0928f7c0 77da7edf 08ef1478 353b3e3a 08ef1578 WTF!WTF::WorkQueue::workThreadCallback+0x20
WARNING: Stack unwind information not available. Following frames may be wrong.
0928f834 77d90951 08ef1478 08ef1578 353b3f8a ntdll_77d40000!RtlClearAllBits+0x110
0928f984 7699336a 00705980 0928f9d0 77d79902 ntdll_77d40000!TpCallbackIndependent+0x710
0928f990 77d79902 00705980 353b3fde 00000000 KERNEL32!BaseThreadInitThunk+0x12
0928f9d0 77d798d5 77d9046c 00705980 00000000 ntdll_77d40000!RtlInitializeExceptionChain+0x63
0928f9e8 00000000 77d9046c 00705980 00000000 ntdll_77d40000!RtlInitializeExceptionChain+0x36

[Per Arne Vollan]

Created attachment 319280 [details]
Patch

[Brent Fulgham]

Comment on attachment 319280 [details]
Patch

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

> Source/WTF/wtf/win/WorkQueueWin.cpp:62
> +                function();

I think we should understand how a nullptr function is getting into this Vector. Maybe we could just protect against nullptr functions being placed in the queue? We don't seem to do this kind of null-check elsewhere in the code, so it seems like an anti-pattern.

[Per Arne Vollan]

Created attachment 319283 [details]
Patch

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #2)
> Comment on attachment 319280 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=319280&action=review
> 
> > Source/WTF/wtf/win/WorkQueueWin.cpp:62
> > +                function();
> 
> I think we should understand how a nullptr function is getting into this
> Vector. Maybe we could just protect against nullptr functions being placed
> in the queue? We don't seem to do this kind of null-check elsewhere in the
> code, so it seems like an anti-pattern.

Thanks! That's a good point, I have updated the patch.

[Saam Barati]

Comment on attachment 319283 [details]
Patch

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

> Source/WTF/wtf/win/WorkQueueWin.cpp:105
> +    ASSERT(function);
> +    if (!function)
> +        return;

Maybe this should be accompanied with a FIXME and a bug to fix the underlying issue?

[Per Arne Vollan]

Created attachment 319291 [details]
Patch

[Per Arne Vollan]

(In reply to Saam Barati from comment #5)
> Comment on attachment 319283 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=319283&action=review
> 
> > Source/WTF/wtf/win/WorkQueueWin.cpp:105
> > +    ASSERT(function);
> > +    if (!function)
> > +        return;
> 
> Maybe this should be accompanied with a FIXME and a bug to fix the
> underlying issue?

Yes, thank you! I have updated the patch.

[WebKit Commit Bot]

Comment on attachment 319291 [details]
Patch

Clearing flags on attachment: 319291

Committed r221323: <http://trac.webkit.org/changeset/221323>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34146574>