Bug 172797 – Make WebCore::defaultPortForProtocol() thread-safe

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

[Brent Fulgham]

Accessing WebCore::defaultPortForProtocol from multiple threads is not safe because of the static DefaultPortForProtocolMapForTesting HashMap.

Add an ASSERT to make sure we are not accessing this method from multiple threads.

[Brent Fulgham]

Created attachment 311681 [details]
Patch

[Build Bot]

Comment on attachment 311681 [details]
Patch

Attachment 311681 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3852645

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 311685 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

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

[WebKit Commit Bot]

Comment on attachment 311681 [details]
Patch

Clearing flags on attachment: 311681

Committed r217660: <http://trac.webkit.org/changeset/217660>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

Reverted r217660 for reason:

This should not have landed given that the new assertion is hit on debug EWS bots

Committed r217661: <http://trac.webkit.org/changeset/217661>

[Chris Dumez]

Make WebCore::defaultPortForProtocol() thread-safe since it is called from the SecurityOrigin constructor and SecurityOrigin objects are constructed from various threads.

[Chris Dumez]

Created attachment 311741 [details]
Patch

[Chris Dumez]

Created attachment 311745 [details]
Patch

[Chris Dumez]

Created attachment 311748 [details]
Patch

[Brent Fulgham]

Comment on attachment 311748 [details]
Patch

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

r=me

> Source/WebCore/platform/URL.cpp:547
> +static Lock& defaultPortForProtocolMapLock()

Maybe call this "defaultPortForProtocolMapForTestingLock" to match the other methods?

> Source/WebCore/platform/URL.cpp:556
> +    static DefaultPortForProtocolMapForTesting* defaultPortForProtocolMap;

Do we need to initialize this to nullptr?

[Chris Dumez]

Comment on attachment 311748 [details]
Patch

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

>> Source/WebCore/platform/URL.cpp:547
>> +static Lock& defaultPortForProtocolMapLock()
> 
> Maybe call this "defaultPortForProtocolMapForTestingLock" to match the other methods?

Ok

>> Source/WebCore/platform/URL.cpp:556
>> +    static DefaultPortForProtocolMapForTesting* defaultPortForProtocolMap;
> 
> Do we need to initialize this to nullptr?

This is not needed here. Statics are guaranteed to be implicitly zero-initialized by both C and C++ standards.

[Chris Dumez]

Committed r217682: <http://trac.webkit.org/changeset/217682>