This is a case we claim to support, but it only works if the object has only one reference. In that case, when the user unrefs it, the weak ref notify callback removes the object from the cache. However, if the object has more than one ref, the cache doesn't know the user unreffed it, and when clearing the cache we try to remove more references that what the object actually has, causing a crash in g_object_unref. See the backtrace in bug #118788.
Created attachment 250347 [details] Patch
Comment on attachment 250347 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250347&action=review > Source/WebCore/ChangeLog:13 > + try to remove more references that what the object actually has, Nit: that what -> than what > Source/WebCore/bindings/gobject/DOMObjectCache.cpp:48 > + cacheReferences = std::min(static_cast<unsigned>(object->ref_count), cacheReferences); It might be worth dropping a comment here explaining why cacheReferences might be incorrect at this point.
Comment on attachment 250347 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250347&action=review >> Source/WebCore/bindings/gobject/DOMObjectCache.cpp:48 >> + cacheReferences = std::min(static_cast<unsigned>(object->ref_count), cacheReferences); > > It might be worth dropping a comment here explaining why cacheReferences might be incorrect at this point. Also, coulnd't we completely forget about cacheReferences and simply use GObject's ref_count ?
(In reply to comment #3) > Also, coulnd't we completely forget about cacheReferences and simply use > GObject's ref_count ? I don't think we can, because if the user explicitly refs the object, we should not delete it.
(In reply to comment #4) > (In reply to comment #3) > > > Also, coulnd't we completely forget about cacheReferences and simply use > > GObject's ref_count ? > > I don't think we can, because if the user explicitly refs the object, we > should not delete it. Exactly, cacheReferences only keeps track of the references owned by the cache itself. In a normal case, we just drop our refs, but users can do weird things.
(In reply to comment #2) > Comment on attachment 250347 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=250347&action=review > > > Source/WebCore/ChangeLog:13 > > + try to remove more references that what the object actually has, > > Nit: that what -> than what I always doubt :-P > > Source/WebCore/bindings/gobject/DOMObjectCache.cpp:48 > > + cacheReferences = std::min(static_cast<unsigned>(object->ref_count), cacheReferences); > > It might be worth dropping a comment here explaining why cacheReferences > might be incorrect at this point. Sure!
Committed r182537: <http://trac.webkit.org/changeset/182537>