[webkit-dev] PSA: String shouldn't be a member of a ThreadSafeRefCounted class

[Ryosuke Niwa]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20180223/6b87ff7b/attachment.html>