175064 2017-08-01 20:51:23 -0700 CFString leak dragging an image - allocation under PlatformPasteboard::writeObjectRepresentations 2017-08-02 11:45:04 -0700 1 1 1 Unclassified WebKit WebKit Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck commit-queue darin joepeck thorton webkit-bug-importer wenson_hsieh oldest_to_newest 1334116 0 joepeck 2017-08-01 20:51:23 -0700 Leak seen in an iOS application dragging an image. Leak: 0x1393de630 size=64 zone: WebKit Using System Malloc_0x10315c000 0x39372260 0x00000001 0xc0192550 0x00000001 `"79....P%...... 0x00000000 0x00000000 0xb58c2f31 0x000001a1 ........1/...... 0x00000740 0x00000001 0x39372274 0x00000001 @.......t"79.... 0x0000009a 0x00000000 0x00000000 0x00000000 ................ Call stack: [thread 0x1b58beb40]: | 0x0 | start | 0x102772080 | UIApplicationMain ... | -[WebView(WebPrivate) _requestStartDataInteraction:globalPosition:] | WebCore::EventHandler::tryToBeginDataInteractionAtPoint(WebCore::IntPoint const&, WebCore::IntPoint const&) | WebCore::EventHandler::handleMouseDraggedEvent(WebCore::MouseEventWithHitTestResults const&, WebCore::CheckDragHysteresis) | WebCore::EventHandler::handleDrag(WebCore::MouseEventWithHitTestResults const&, WebCore::CheckDragHysteresis) | WebCore::DragController::startDrag(WebCore::Frame&, WebCore::DragState const&, WebCore::DragOperation, WebCore::PlatformMouseEvent const&, WebCore::IntPoint const&) | WebDragClient::declareAndWriteDragImage(WTF::String const&, WebCore::Element&, WebCore::URL const&, WTF::String const&, WebCore::Frame*) | WebCore::Editor::writeImageToPasteboard(WebCore::Pasteboard&, WebCore::Element&, WebCore::URL const&, WTF::String const&) | non-virtual thunk to WebPlatformStrategies::writeToPasteboard(WebCore::PasteboardImage const&, WTF::String const&) | WebCore::PlatformPasteboard::writeObjectRepresentations(WebCore::PasteboardImage const&) | WTF::StringImpl::operator NSString*() | WTF::StringImpl::createCFString() | CFStringCreateWithBytesNoCopy | __CFStringCreateImmutableFunnel3 | _CFRuntimeCreateInstance | WTF::StringWrapperCFAllocator::allocate(long, unsigned long, void*) | WTF::fastMalloc(unsigned long) | bmalloc::DebugHeap::malloc(unsigned long) | malloc_zone_malloc There are only a few WTF::String -> NSString conversions in this function. Notably: > auto utiOrMIMEType = pasteboardImage.resourceMIMEType.createCFString(); > if (!UTTypeIsDeclared(utiOrMIMEType.get())) > utiOrMIMEType = UTTypeCreatePreferredIdentifierForTag(kUTTagClassMIMEType, utiOrMIMEType.get(), nil); In the first assignment the type is RetainPtr and should be fine. In the second assignment it looks like an extra +1. We should adopt a Created value into a RetainPtr. 1334117 1 joepeck 2017-08-01 20:53:24 -0700 I am able to reproduce, I'll try just adding an adoptCF() here. 1334123 2 316931 joepeck 2017-08-01 22:18:44 -0700 Created attachment 316931 [PATCH] Proposed Fix 1334128 3 316931 thorton 2017-08-01 22:39:38 -0700 Comment on attachment 316931 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=316931&action=review > Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:296 > auto utiOrMIMEType = pasteboardImage.resourceMIMEType.createCFString(); This is one of those times where I really don't like this auto > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:121 > + [_suggestedName release]; Alternatively, we could stop with the autosynthesis and back this with a RetainPtr. 1334137 4 316931 commit-queue 2017-08-01 23:40:42 -0700 Comment on attachment 316931 [PATCH] Proposed Fix Clearing flags on attachment: 316931 Committed r220124: <http://trac.webkit.org/changeset/220124> 1334138 5 commit-queue 2017-08-01 23:40:44 -0700 All reviewed patches have been landed. Closing bug. 1334139 6 webkit-bug-importer 2017-08-01 23:41:24 -0700 <rdar://problem/33671119> 1334167 7 316931 joepeck 2017-08-02 01:22:02 -0700 Comment on attachment 316931 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=316931&action=review >> Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:121 >> + [_suggestedName release]; > > Alternatively, we could stop with the autosynthesis and back this with a RetainPtr. Yeah, I normally would do that but I was confused by what RetainPtr would do given we specified the property as -copy. Would it just work, or would it actually be doing -retains under the hood unless we implemented our own setter that explicitly called -copy. Do you happen to know what it would do? 1334214 8 thorton 2017-08-02 08:19:02 -0700 You would need to copy yourself. 1334217 9 316931 darin 2017-08-02 08:26:08 -0700 Comment on attachment 316931 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=316931&action=review >>> Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:121 >>> + [_suggestedName release]; >> >> Alternatively, we could stop with the autosynthesis and back this with a RetainPtr. > > Yeah, I normally would do that but I was confused by what RetainPtr would do given we specified the property as -copy. Would it just work, or would it actually be doing -retains under the hood unless we implemented our own setter that explicitly called -copy. Do you happen to know what it would do? RetainPtr itself definitely does not know how to copy rather than retain. But you would write code like this: _suggestedName = adoptNS([suggestedName copy]); And that’s how you would get it to copy. 1334324 10 joepeck 2017-08-02 11:45:04 -0700 Okay, so you'd have to write out the setter. Thats what I figured. 316931 2017-08-01 22:18:44 -0700 2017-08-01 23:40:42 -0700 [PATCH] Proposed Fix leaks-2.patch text/plain 3406 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAzYmI2NmVmNzUxNC4uZWFkZTNlNjYyZTMgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxOSBAQAorMjAxNy0wOC0wMSAgSm9zZXBoIFBlY29yYXJvICA8cGVjb3Jhcm9AYXBwbGUu Y29tPgorCisgICAgICAgIENGU3RyaW5nIGxlYWsgZHJhZ2dpbmcgYW4gaW1hZ2UgLSBhbGxvY2F0 aW9uIHVuZGVyIFBsYXRmb3JtUGFzdGVib2FyZDo6d3JpdGVPYmplY3RSZXByZXNlbnRhdGlvbnMK KyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE3NTA2NAor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogcGxhdGZv cm0vaW9zL1BsYXRmb3JtUGFzdGVib2FyZElPUy5tbToKKyAgICAgICAgKFdlYkNvcmU6OlBsYXRm b3JtUGFzdGVib2FyZDo6d3JpdGVPYmplY3RSZXByZXNlbnRhdGlvbnMpOgorICAgICAgICBBZG9w dCBhIGNyZWF0ZWQgc3RyaW5nIGludG8gdGhlIFJldGFpblB0ci4KKworICAgICAgICAqIHBsYXRm b3JtL2lvcy9XZWJJdGVtUHJvdmlkZXJQYXN0ZWJvYXJkLmg6CisgICAgICAgICogcGxhdGZvcm0v aW9zL1dlYkl0ZW1Qcm92aWRlclBhc3RlYm9hcmQubW06CisgICAgICAgICgtW1dlYkl0ZW1Qcm92 aWRlclJlZ2lzdHJhdGlvbkluZm9MaXN0IGRlYWxsb2NdKToKKyAgICAgICAgUmVsZWFzZSBzdWdn ZXN0ZWROYW1lIGFuZCBzd2l0Y2ggZnJvbSAtc3Ryb25nIHRvIC1jb3B5LgorCiAyMDE3LTA3LTI2 ICBKaWV3ZW4gVGFuICA8amlld2VuX3RhbkBhcHBsZS5jb20+CiAKICAgICAgICAgQWRkIHRlc3Rz IHRvIGRldGVjdCBtaXN0YWtlcyBpbiBiYWNrd2FyZCBjb21wYXRpYmlsaXR5IHdoZW4gdGhlIHN0 cnVjdHVyZWQgY2xvbmUgYWxnb3JpdGhtIGlzIGNoYW5nZWQgaW4gdGhlIGZ1dHVyZQpkaWZmIC0t Z2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vaW9zL1BsYXRmb3JtUGFzdGVib2FyZElPUy5t bSBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2lvcy9QbGF0Zm9ybVBhc3RlYm9hcmRJT1MubW0K aW5kZXggZTg4ZGRlODk2YmMuLmM1ZWU1YmEzMTA3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9pb3MvUGxhdGZvcm1QYXN0ZWJvYXJkSU9TLm1tCisrKyBiL1NvdXJjZS9XZWJD b3JlL3BsYXRmb3JtL2lvcy9QbGF0Zm9ybVBhc3RlYm9hcmRJT1MubW0KQEAgLTI5NSw3ICsyOTUs NyBAQCB2b2lkIFBsYXRmb3JtUGFzdGVib2FyZDo6d3JpdGVPYmplY3RSZXByZXNlbnRhdGlvbnMo Y29uc3QgUGFzdGVib2FyZEltYWdlJiBwYXN0ZQogICAgIGlmIChwYXN0ZWJvYXJkSW1hZ2UucmVz b3VyY2VEYXRhICYmICFwYXN0ZWJvYXJkSW1hZ2UucmVzb3VyY2VNSU1FVHlwZS5pc0VtcHR5KCkp IHsKICAgICAgICAgYXV0byB1dGlPck1JTUVUeXBlID0gcGFzdGVib2FyZEltYWdlLnJlc291cmNl TUlNRVR5cGUuY3JlYXRlQ0ZTdHJpbmcoKTsKICAgICAgICAgaWYgKCFVVFR5cGVJc0RlY2xhcmVk KHV0aU9yTUlNRVR5cGUuZ2V0KCkpKQotICAgICAgICAgICAgdXRpT3JNSU1FVHlwZSA9IFVUVHlw ZUNyZWF0ZVByZWZlcnJlZElkZW50aWZpZXJGb3JUYWcoa1VUVGFnQ2xhc3NNSU1FVHlwZSwgdXRp T3JNSU1FVHlwZS5nZXQoKSwgbmlsKTsKKyAgICAgICAgICAgIHV0aU9yTUlNRVR5cGUgPSBhZG9w dENGKFVUVHlwZUNyZWF0ZVByZWZlcnJlZElkZW50aWZpZXJGb3JUYWcoa1VUVGFnQ2xhc3NNSU1F VHlwZSwgdXRpT3JNSU1FVHlwZS5nZXQoKSwgbmlsKSk7CiAKICAgICAgICAgYXV0byBpbWFnZURh dGEgPSBwYXN0ZWJvYXJkSW1hZ2UucmVzb3VyY2VEYXRhLT5jcmVhdGVOU0RhdGEoKTsKICAgICAg ICAgW2l0ZW1zVG9SZWdpc3RlciBhZGREYXRhOmltYWdlRGF0YS5nZXQoKSBmb3JUeXBlOihOU1N0 cmluZyAqKXV0aU9yTUlNRVR5cGUuZ2V0KCldOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vaW9zL1dlYkl0ZW1Qcm92aWRlclBhc3RlYm9hcmQuaCBiL1NvdXJjZS9XZWJDb3Jl L3BsYXRmb3JtL2lvcy9XZWJJdGVtUHJvdmlkZXJQYXN0ZWJvYXJkLmgKaW5kZXggMzA0ODM3MWNi MjcuLjNhNGI2ZmVkYjY5IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9pb3Mv V2ViSXRlbVByb3ZpZGVyUGFzdGVib2FyZC5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L2lvcy9XZWJJdGVtUHJvdmlkZXJQYXN0ZWJvYXJkLmgKQEAgLTYxLDcgKzYxLDcgQEAgV0VCQ09S RV9FWFBPUlQgQGludGVyZmFjZSBXZWJJdGVtUHJvdmlkZXJSZWdpc3RyYXRpb25JbmZvTGlzdCA6 IE5TT2JqZWN0CiAtICh2b2lkKWFkZERhdGE6KE5TRGF0YSAqKWRhdGEgZm9yVHlwZTooTlNTdHJp bmcgKil0eXBlSWRlbnRpZmllcjsKIAogQHByb3BlcnR5IChub25hdG9taWMpIENHU2l6ZSBlc3Rp bWF0ZWREaXNwbGF5ZWRTaXplOwotQHByb3BlcnR5IChub25hdG9taWMsIHN0cm9uZykgTlNTdHJp bmcgKnN1Z2dlc3RlZE5hbWU7CitAcHJvcGVydHkgKG5vbmF0b21pYywgY29weSkgTlNTdHJpbmcg KnN1Z2dlc3RlZE5hbWU7CiAKIC0gKE5TVUludGVnZXIpbnVtYmVyT2ZJdGVtczsKIC0gKG51bGxh YmxlIFdlYkl0ZW1Qcm92aWRlclJlZ2lzdHJhdGlvbkluZm8gKilpdGVtQXRJbmRleDooTlNVSW50 ZWdlcilpbmRleDsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2lvcy9XZWJJ dGVtUHJvdmlkZXJQYXN0ZWJvYXJkLm1tIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vaW9zL1dl Ykl0ZW1Qcm92aWRlclBhc3RlYm9hcmQubW0KaW5kZXggNmM2MmIyZGM2ZGIuLjA1Y2JjMTg4YjA5 IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9pb3MvV2ViSXRlbVByb3ZpZGVy UGFzdGVib2FyZC5tbQorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9pb3MvV2ViSXRlbVBy b3ZpZGVyUGFzdGVib2FyZC5tbQpAQCAtMTE2LDYgKzExNiwxMiBAQCAtIChpbnN0YW5jZXR5cGUp aW5pdAogICAgIHJldHVybiBzZWxmOwogfQogCistICh2b2lkKWRlYWxsb2MKK3sKKyAgICBbX3N1 Z2dlc3RlZE5hbWUgcmVsZWFzZV07CisgICAgW3N1cGVyIGRlYWxsb2NdOworfQorCiAtICh2b2lk KWFkZERhdGE6KE5TRGF0YSAqKWRhdGEgZm9yVHlwZTooTlNTdHJpbmcgKil0eXBlSWRlbnRpZmll cgogewogICAgIFtfaXRlbXMgYWRkT2JqZWN0OltbW1dlYkl0ZW1Qcm92aWRlclJlZ2lzdHJhdGlv bkluZm8gYWxsb2NdIGluaXRXaXRoUmVwcmVzZW50aW5nT2JqZWN0Om5pbCB0eXBlSWRlbnRpZmll cjp0eXBlSWRlbnRpZmllciBkYXRhOmRhdGFdIGF1dG9yZWxlYXNlXV07Cg==