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.

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

[SR-14419] Clarify guarantees around the lifetime of self in member functions #56775

Open
Lukasa opened this issue Mar 29, 2021 · 4 comments
Open
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself

Comments

@Lukasa
Copy link
Contributor

Lukasa commented Mar 29, 2021

Previous ID SR-14419
Radar rdar://35924668
Original Reporter @Lukasa
Type Bug
Additional Detail from JIRA
Votes 2
Component/s Compiler
Labels Bug
Assignee None
Priority Medium

md5: b64f2a4515b49665921f4ac37c92be2a

Issue Description:

Recent versions of Swift have a more effective Copy Propagation pass that much more aggressively shortens the lifetime of some Swift objects. This has revealed some latent bugs in several projects, including [swift-corelibs-foundation| apple/swift-corelibs-foundation#2994].

In general, Swift has a very clear rule about lifetime: an object can be deallocated at any time after its last point of use. This is a clear guide, and easy enough to follow. However, there is a more subtle case: how does this apply to self in member functions?

To make this concrete we can consider a (contrived) example:

public final class WeirdFileThingy {
    private let fd: CInt
    init(_ x: CInt) {
        fd = x
    }
    deinit {
        close(fd)
    }
    public func read() -> UnsafeMutableRawBufferPointer {
        let localFD = self.fd
        var data = UnsafeMutableRawPointer.allocate(count: 100, alignment: 1)
        let length = data.withUnsafeMutableBytes {
            Darwin.read(localFD, $0.baseAddress, 100)
        }
        precondition(length != -1 || errno != EBADF)
        return UnsafeMutableRawBufferPointer(start: data, count: length)
    }
}

The question is, can the call to Darwin.read fail with EBADF and hit the trap on the following line? The answer to this question will depend on whether the deinit for this class is allowed to execute after the let localFD = self.fd line (the last use of self in this function), or whether self is guaranteed to be at +1 until the end of the member function.

As @gottesmm has noted, there are two vectors to this. The first is the question of whether this code is safe today. Areas of particular concern are around inlining. The second is the question of whether Swift should formally guarantee that self is always retained throughout the entirety of a member function invocation. Today I don’t think we have clear guidance for users as to what the lifetime of self is in this situation, and I think we should come to a clear guarantee one way or another.

@weissi
Copy link
Member

weissi commented Apr 7, 2021

@swift-ci create

@weissi
Copy link
Member

weissi commented Apr 7, 2021

CC @DougGregor is this actually not formalised?

@atrick
Copy link
Member

atrick commented Apr 7, 2021

The sample code above should be valid once we formalize the language. Today, the read method requires withExtendedLifetime(self) or _fixLifetime to guard against what I consider to be compiler bugs. That workaround is overly conservative.

The language rule should be:

✔ self will be alive within the body of read as long as any value derived from self is used within read.

The language rule should not be:

❌ self will be alive within the body of read. That would require inserting a lifetime marker (equivalent to _fixLifetime) for all methods, making it impossible to optimize dead methods. That is really out of the question

The language rule should not be:

❌ self will be alive within the body of read until the last use of self. While this would be consistent with lifetime rules outside of method invocation, it would violate obvious programmer expectations, be a perpetual source of bugs, and regularly require a conservative _fixLifetime workarounds like we currently see all over the stdlib.

@atrick
Copy link
Member

atrick commented Apr 7, 2021

As far as what is specified today, the language is deliberately vague forcing programmers to assume worst case. So pedantically, the current compiler optimizations are not incorrect. But they are still wrong in spirit.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself
Projects
None yet
Development

No branches or pull requests

3 participants