Bug 199626 – Fix non thread-safe use of WeakPtr in DisplayRefreshMonitorMac::displayLinkFired()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Fix non thread-safe use of WeakPtr in DisplayRefreshMonitorMac::displayLinkFired():
Thread 11 Crashed:: CVDisplayLink
0   com.apple.JavaScriptCore      	0x000000010e6a837e WTFCrash + 14 (Assertions.cpp:305)
1   com.apple.WebCore             	0x0000000123e867eb WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.WebCore             	0x000000012734032c WebCore::DisplayRefreshMonitorMac::WeakValueType* WTF::WeakPtrImpl::get<WebCore::DisplayRefreshMonitorMac>() + 140 (WeakPtr.h:65)
3   com.apple.WebCore             	0x00000001273401aa WTF::WeakPtrFactory<WebCore::DisplayRefreshMonitorMac>::createWeakPtr(WebCore::DisplayRefreshMonitorMac&) const + 138 (WeakPtr.h:142)
4   com.apple.WebCore             	0x000000012733dab7 WTF::WeakPtr<WebCore::DisplayRefreshMonitorMac> WTF::makeWeakPtr<WebCore::DisplayRefreshMonitorMac>(WebCore::DisplayRefreshMonitorMac&) + 55 (WeakPtr.h:212)
5   com.apple.WebCore             	0x000000012733d9ef WebCore::DisplayRefreshMonitorMac::displayLinkFired() + 111 (DisplayRefreshMonitorMac.cpp:92)
6   com.apple.WebCore             	0x000000012733d935 WebCore::displayLinkCallback(__CVDisplayLink*, CVTimeStamp const*, CVTimeStamp const*, unsigned long long, unsigned long long*, void*) + 53 (DisplayRefreshMonitorMac.cpp:53)
7   com.apple.CoreVideo           	0x00007fff3833fe16 CVDisplayLink::performIO(CVTimeStamp*) + 230
8   com.apple.CoreVideo           	0x00007fff3833f2ee CVDisplayLink::runIOThread() + 626
9   libsystem_pthread.dylib       	0x00007fff6dd36daa _pthread_start + 125
10  libsystem_pthread.dylib       	0x00007fff6dd336af thread_start + 15

[Chris Dumez]

Created attachment 373731 [details]
Patch

[EWS Watchlist]

Attachment 373731 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/DisplayRefreshMonitor.h:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

Comment on attachment 373731 [details]
Patch

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

Maybe we need ThreadSafeWeakPtr...

> Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.h:46
>      void displayLinkFired() override;

This does class need to inherit from CanMakeWeakPtr now?

[Chris Dumez]

(In reply to Ryosuke Niwa from comment #3)
> Comment on attachment 373731 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=373731&action=review
> 
> Maybe we need ThreadSafeWeakPtr...
> 
> > Source/WebCore/platform/graphics/mac/DisplayRefreshMonitorMac.h:46
> >      void displayLinkFired() override;
> 
> This does class need to inherit from CanMakeWeakPtr now?

Oh, it likely doesn't. Good catch.

[Chris Dumez]

Created attachment 373733 [details]
Patch

[EWS Watchlist]

Attachment 373733 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/DisplayRefreshMonitor.h:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 373733 [details]
Patch

Attachment 373733 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12699554

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 373736 [details]
Archive of layout-test-results from ews117 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Chris Dumez]

Created attachment 373737 [details]
Patch

[EWS Watchlist]

Attachment 373737 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/DisplayRefreshMonitor.h:33:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp:112:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 373737 [details]
Patch

Clearing flags on attachment: 373737

Committed r247273: <https://trac.webkit.org/changeset/247273>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/52848623>