Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SR-6849] Data leaks on Linux in Swift 4.1 #3741

Closed
Lukasa opened this issue Jan 26, 2018 · 9 comments
Closed

[SR-6849] Data leaks on Linux in Swift 4.1 #3741

Lukasa opened this issue Jan 26, 2018 · 9 comments
Assignees
@phausler
Labels
bug Foundation

Comments

@Lukasa
Copy link
Contributor

Lukasa commented Jan 26, 2018

Previous ID SR-6849
Radar rdar://problem/36905620
Original Reporter @Lukasa
Type Bug
Status Closed
Resolution Done
Environment

Ubuntu 16.04
Swift version 4.1-dev (LLVM 5b54bd1e96, Clang 03ed64977b, Swift 88a7a55e83)
Target: x86_64-unknown-linux-gnu

Additional Detail from JIRA
Votes 1
Component/s Foundation
Labels Bug
Assignee @phausler
Priority Medium

md5: 565e16afee84c6e52512f27cc992472d

Issue Description:

The following program fails under ASAN on Linux when using the Swift nightly from January 25:

import Foundation

for _ in 0..<10 {
        let data = Data(base64Encoded: "UE9TVCAvIEhUVFAvMS4wDQpDb250ZW50LUxlbmd0aDoNCg0K")!
}

The output is:

=================================================================
==7043==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 360 byte(s) in 10 object(s) allocated from:
    #&#8203;0 0x556f77e99d63  (/home/cbenfield/test+0x9dd63)
    #&#8203;1 0x7ff2c57bbbb2  (/usr/local/swift/swift-4.1-DEVELOPMENT-SNAPSHOT-2018-01-25-a-ubuntu16.04/usr/lib/swift/linux/libFoundation.so+0x22fbb2)

SUMMARY: AddressSanitizer: 360 byte(s) leaked in 10 allocation(s).

If we want a more helpful stack we can disable ASAN and run under Valgrind:

==7078== 360 bytes in 10 blocks are definitely lost in loss record 15 of 19
==7078==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7078==    by 0x553CBB2: _CFDataInit (in /usr/local/swift/swift-4.1-DEVELOPMENT-SNAPSHOT-2018-01-25-a-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==7078==    by 0x56EB42A: function signature specialization <Arg[0] = Exploded> of Foundation.NSData.init(base64Encoded: Swift.String, options: Foundation.NSData.Base64DecodingOptions) -> Foundation.NSData? (in /usr/local/swift/swift-4.1-DEVELOPMENT-SNAPSHOT-2018-01-25-a-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==7078==    by 0x56E2437: merged Foundation.NSData.__allocating_init(base64Encoded: Swift.String, options: Foundation.NSData.Base64DecodingOptions) -> Foundation.NSData? (in /usr/local/swift/swift-4.1-DEVELOPMENT-SNAPSHOT-2018-01-25-a-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==7078==    by 0x56E239F: Foundation.NSData.__allocating_init(base64Encoded: Swift.String, options: Foundation.NSData.Base64DecodingOptions) -> Foundation.NSData? (in /usr/local/swift/swift-4.1-DEVELOPMENT-SNAPSHOT-2018-01-25-a-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==7078==    by 0x59A6972: Foundation.Data.init(base64Encoded: Swift.String, options: Foundation.NSData.Base64DecodingOptions) -> Foundation.Data? (in /usr/local/swift/swift-4.1-DEVELOPMENT-SNAPSHOT-2018-01-25-a-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==7078==    by 0x1090A2: main (in /home/cbenfield/test)
==7078==
==7078== LEAK SUMMARY:
==7078==    definitely lost: 360 bytes in 10 blocks

This just seems like it's a straightforward leak of CFData.
@Lukasa
Copy link
Contributor Author

Lukasa commented Jan 26, 2018

The base64 decoding seems to be at fault here: changing the initialiser to just init from [UInt8] removes the leak.

@alblue
Copy link
Contributor

alblue commented Jan 26, 2018

We triaged this bug to the following commit:

df3ec55

This is the mega-commit from the changes in High Sierra.

In particular, this seems to have changed flags for __kCFDontDeallocate and friends from a bitmask to individual integers. However, the versions imported into Swift don't appear to have changed:

swift-corelibs-foundation/Foundation/NSData.swift

Lines 70 to 76 in 7157437

private let __kCFMutable: CFOptionFlags = 0x01
private let __kCFGrowable: CFOptionFlags = 0x02
private let __kCFMutableVarietyMask: CFOptionFlags = 0x03
private let __kCFBytesInline: CFOptionFlags = 0x04
private let __kCFUseAllocator: CFOptionFlags = 0x08
private let __kCFDontDeallocate: CFOptionFlags = 0x10
private let __kCFAllocatesCollectable: CFOptionFlags = 0x20

For example, the __kCFDontDeallocate has the value 0x10 in NSData.swift, but in the CFData it is now defined as 4.

It's not clear what the problem is or whether it's directly related, but the options are still being masked together in the Swift codebase which no longer seems to make sense.

@alblue
Copy link
Contributor

alblue commented Jan 26, 2018

@swift-ci create

@swift-ci
Copy link
Contributor

swift-ci commented Feb 6, 2018

Comment by David Jones (JIRA)

I recently tried running Kitura on recent 4.1 and master snapshots and also noticed a leak. Probably the same issue as this (but let me know if you'd like me to raise a separate one). Extract from a valgrind massif report:

93.39% (7,852,969B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc.
->69.76% (5,866,048B) 0x552B7FF: __CFSafelyReallocate (CFBase.c:771)
| ->69.76% (5,866,048B) 0x55589FA: __CFDataGrow (CFData.c:554)
|   ->69.76% (5,866,048B) 0x5556F41: CFDataReplaceBytes (CFData.c:639)
|     ->69.76% (5,866,048B) 0x5558C1D: CFDataAppendBytes (CFData.c:609)
|       ->69.76% (5,866,048B) 0x580576B: Foundation.NSMutableData.append(Swift.UnsafeRawPointer, length: Swift.Int) -> () (NSData.swift:1059)
|         ->69.56% (5,849,088B) 0x241742: KituraNet.HTTPServerResponse.(writeToSocketThroughBuffer in _8A9E8F62631F03CCFCED4F4223E40C30)(text: Swift.String) throws -> () (HTTPServerResponse.swift:204)
...etc

KituraNet code here is: https://github.com/IBM-Swift/Kitura-net/blob/2.0.1/Sources/KituraNet/HTTP/HTTPServerResponse.swift#L204

@Lukasa
Copy link
Contributor Author

Lukasa commented Feb 6, 2018

That looks like a different leak, I think.

@swift-ci
Copy link
Contributor

swift-ci commented Feb 9, 2018

Comment by David Jones (JIRA)

Raised SR-6962 for the NSMutableData leak.

@swift-ci
Copy link
Contributor

Comment by Mamatha Busi (JIRA)

#380

The above PR had a fix for NSData leak where __CFDataGetInfoBit for __kCFDontDeallocate (then equivalent of __CFDataDontDeallocate before PR for High Sierra changes) was set on an ‘if’ condition and it was in turn checked in the __CFDataDeallocate method of CFData.

However, with the HighSierra PR 1338 changes now, the value for __CFDataDontDeallocate is hardcoded to 'true'.

__CFDataSetDontDeallocate(memory, true);

https://github.com/apple/swift-corelibs-foundation/blob/master/CoreFoundation/Collections.subproj/CFData.c

Therefore, in the __CFDataDeallocate method of CFData, deallocation of CFData never happens. This is causing the leak. IMO, conditional check for setting _CFDataDeallocate based on the CFOptionFlags needs to be introduced again. Could you please have a look at my analysis @phausler

@swift-ci
Copy link
Contributor

Comment by Mamatha Busi (JIRA)

Opened PR-#1455

@Lukasa
Copy link
Contributor Author

Lukasa commented Apr 26, 2018

This appears to be fixed.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@phausler phausler

Labels
bug Foundation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@alblue @Lukasa @swift-ci