-
Notifications
You must be signed in to change notification settings - Fork 479
don't hold references to unowned DispatchData objects (SR-3628) #200
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.
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
Conversation
|
borrowedData was added to Data to compensate for memory leaks that I think were related to iteration. I will go back and look at that PR / SR to see how this proposed change interacts with that. |
|
ok. This change makes sense to me. When I "fixed" the over-release problem of SR-2656 by introducing borrowedData: in #175 I didn't account for when the __DispatchData that was borrowing the dispatch_data_t outlived the "real" owner. I was too focused on trying to avoid the overhead of an "extra" retain/release operation. But, it really can't be avoided (as shown by the test case for this bug). |
| return unsafeBitCast(__wrapped, to: dispatch_object_t.self) | ||
| } | ||
|
|
||
| internal init(data:dispatch_data_t, owned:Bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that owned tag should really be transferOwnership: Bool but the patch LGTM and I should have realized when we reviewed the earlier patch :/
|
@swift-ci please test |
|
build seems healthy to me but then failed because of
|
|
@swift-ci please test |
|
@dgrove-oss I'm not 100% sure, do we need this for another branch than master? we want that for whatever the next swift 3 update will be. |
|
We should get it into the swift-3.1-branch. I don't think it is worth putting into the swift-3.0-branch. |
|
@dgrove-oss sure, on it! |
don't hold references to unowned DispatchData objects (SR-3628) Signed-off-by: Daniel A. Steffen <dsteffen@apple.com>
In
DispatchIO.read, thedispatch_data_tobject received fromdispatch_io_readwill be released once the block passed todispatch_io_readreturns.In the Swift overlay (
src/swift/IO.swift) the code marks that data object correctly as "borrowedData"that however leads to use-after-frees if the Swift
DispatchDatais kept around for longer.The change I made will
retainthe underlyingdispatch_data_tif it's "borrowed" which I believe is correct and fixes SR-3628.