Bug 133936 – API::String is not thread-safe

• Status: NEW
• Component: WebKit2

API::String is not thread-safe

Created attachment 233152 [details]
Safely dispose of m_string on the main thread

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/Shared/APIString.h:133:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 3 files


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

[Anders Carlsson]

Comment on attachment 233152 [details]
Safely dispose of m_string on the main thread

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

> Source/WebKit2/Shared/APIString.h:68
> +        WTF::String string = std::move(m_string);

Can use auto here.

> Source/WebKit2/Shared/APIString.h:69
> +        RunLoop::main().dispatch([string]() { });

I think this needs a comment.

> Source/WebKit2/Shared/APIString.h:115
> +        RunLoop::main().dispatch([string]() { });

I think this needs a comment.

Created attachment 233218 [details]
Safely dispose of m_string on the main thread

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/Shared/APIString.h:135:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 3 files


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

[Anders Carlsson]

Comment on attachment 233218 [details]
Safely dispose of m_string on the main thread

What if the string is created off the main thread?

(In reply to comment #6)
> (From update of attachment 233218 [details])
> What if the string is created off the main thread?

In that case we make an isolated copy at creation time, so we are free to dispose of it on the main thread eventually. The invariant is that m_string is always safe to destroy on the main thread.

Oh, I think you're saying that the string() accessor may violate the invariant. It should just make an isolated copy and return it whenever it's called off the main thread.

The fix for bug 139261 made API::String copy unconditionally on creation.