[Webkit-unassigned] [Bug 193291] Leak of WTF::StringImpl under SymbolImpl::createNullSymbol() (48 bytes) in com.apple.WebKit.WebContent running layout tests

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=193291

--- Comment #9 from Yusuke Suzuki <yusukesuzuki at slowstart.org> ---
Comment on attachment 358722
  --> https://bugs.webkit.org/attachment.cgi?id=358722
Patch v1

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

>>> Source/WTF/wtf/text/SymbolImpl.cpp:59
>>> +    return nullSymbol.get().get();
>> 
>> NOTE: I've confirmed that this DOES fix the leak, but I'm not sure exactly why (so it might be papering over the root cause).  I need to switch back to another task, so I'm posting this in case it's actually the correct solution and the reviewer is confident of that.
> 
> Oh, why is SymbolImpl::m_owner a raw pointer instead of a RefPtr<StringImpl>?
> 
>     // The pointer to the owner string should be immediately following after the StringImpl layout,
>     // since we would like to align the layout of SymbolImpl to the one of BufferSubstring StringImpl.
>     StringImpl* m_owner;
> 
> And there is no SymbolImpl:~:SymbolImpl() destructor, so I'm not sure what the ownership model of SymbolImpl::m_owner is (for the null symbol or the non-null symbol case).
> 
> That's probably the right question to answer to fix this bug.

This is a bit tricky, but I think it works. All the SymbolImpl should have BufferSubstring buffer ownership. Then, the location `m_owner` should be the same to `substringBuffer()` (tailPointer<StringImpl*>(), it is tested by ASSERT).
Let's see StringImpl::~StringImpl. It has `->deref()` code for BufferSubstring, and it should release it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190109/b28620c1/attachment.html>