Actually, it is my understanding that isolatedCopy() does the right thing here.
If you look at the implementation:
String String::isolatedCopy() const &
{
// FIXME: Should this function, and the many others like it, be inlined?
return m_impl ? m_impl->isolatedCopy() : String { };
}
String String::isolatedCopy() &&
{
if (isSafeToSendToAnotherThread()) {
// Since we know that our string is a temporary that will be destroyed
// we can just steal the m_impl from it, thus avoiding a copy.
return { WTFMove(*this) };
}
return m_impl ? m_impl->isolatedCopy() : String { };
}
The isolatedCopy() optimization (i.e. isSafeToSendToAnotherThread) can only be
used if the String is a temporary. If you example, m_name is not a temporary
and will be deep copied.
--
Chris Dumez
> On Feb 23, 2018, at 11:45 PM, Ryosuke Niwa <[email protected]> wrote:
>
> Hi all,
>
> This is a remainder that our String class is NOT thread safe, and should NOT
> be used inside an object shared across multiple threads. In particular, it's
> not necessarily safe to have it as a member of ThreadSafeRefCounted class,
> which can be accessed from multiple threads.
>
> Let's consider the following example.
>
> class A : public ThreadSafeRefCounted<A> {
> public:
> A(const String& name)
> : m_name(name)
> { }
> String name() { return m_name.isolatedCopy(); }
>
> private:
> String m_name;
> }
>
> This code is NOT thread safe depending on how name() is used.
>
> For example, if it's ever inserted or looked up in a hash table as the key,
> or if it's ever converted into an AtomicString, then it would lead to memory
> corruption. This is because String::hash() would mutate m_hashAndFlags member
> variable without any lock, and isolatedCopy() doesn't make a copy if there is
> exactly one reference to a given StringImpl (String is basically just a
> RefPtr of StringImpl).
>
> - R. Niwa
> _______________________________________________
> webkit-dev mailing list
> [email protected]
> https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________
webkit-dev mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-dev
