Semantics of += on class type

I recently ran into a very difficult to debug issue in a project I was working on where the use of += was forcing a class to be deinitialized early and this was causing cascading bugs since the deinit closure also closed an open file descriptor.

Class:

final class Open<PathType: Openable>: Opened {
    let fileDescriptor: Int32
    // Blah blah blah

    deinit { PathType.close(descriptor: fileDescriptor) }
}

extension Open where PathType == FilePath {
    public var offset: OSOffsetInt {
        // Calling the C seek function returns the current file offset for the file descriptor.
        // Needed to do this since extensions can't have stored properties and I didn't 
        // wanna have a different `Open` type for every path type that can be opened
        get { return (try? seek(fromCurrent: 0)) ?? -1 }
        nonmutating set { let _ = try? seek(fromStart: newValue) }
    }

    // Seeking functions...
}

Unit test:

func testSeek() {
    let openFile: OpenFile
    // Open the file and write "Hello World" to it

    // Offset is not 0 because writing to the file moves the files offset
    XCTAssertNotEqual(0, openFile.offset)
    XCTAssertNoThrow(try openFile.rewind())
    XCTAssertEqual(0, openFile.offset)

    XCTAssertNoThrow(try openFile.seek(fromEnd: -6))
    XCTAssertEqual(openFile.offset, openFile.size - 6)

    // This call right here causes openFile to be deinitialized. Verified with breakpoints in the Xcode debugger
    openFile.offset += 11

    // Since getting the offset uses the file descriptor to call the C function,
    // this fails once the openFile object has been deinitialized
    // (Getting the file size also would fail since it makes the stat(2) C call
    // using the file descriptor as well
    XCTAssertEqual(openFile.offset, openFile.size - 6 + 11)
}

If I change the line to:

openFile.offset = openFile.offset + 11

Then everything works just fine. I had (apparently wrongfully) assumed that += was basically treated the same.

Would someone be able to explain to me why the += version is causing my class to be deinitialized? It seems to me like a bug in the ARC system since clearly not all references to openFile were gone and so it should never have called the deinit closure.

2 Likes

This sounds like it could be an other occurrence of [SR-8990] Miscompile (possible assertion failure) modifying nonmutating property on protocol · Issue #51493 · apple/swift · GitHub, which I'm investigating now. It's definitely a bug, regardless.

3 Likes

@Ponyboy47 If you try compiling with a snapshot toolchain (either the "4.2 convergence" or "trunk development" toolchain from Swift.org - Download Swift), do you see an assertion failure in the compiler while building your code?

Also, in your code, is Open a class or a protocol? You ought to get an error declaring a nonmutating method on a class extension, since all properties on classes are nonmutating.

Open is a final class.

When I build using the convergence snapshot (on both Ubuntu 18.04 and macOS 10.14) I get the following error:
Assertion failed: (cleanup.isActive() && "forwarding inactive or dead cleanup?"), function forwardCleanup, file /Users/buildnode/jenkins/workspace/oss-swift-4.2-package-osx/swift/lib/SILGen/Cleanup.cpp, line 168.
0  swift                    0x00000001101a3108 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
1  swift                    0x00000001101a2347 llvm::sys::RunSignalHandlers() + 39
2  swift                    0x00000001101a3782 SignalHandler(int) + 258
3  libsystem_platform.dylib 0x00007fff61c25b3d _sigtramp + 29
4  libc++.1.dylib           0x00007fff5f10f53e std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::push_back(char) + 86
5  libsystem_c.dylib        0x00007fff61ae41c9 abort + 127
6  libsystem_c.dylib        0x00007fff61aac868 basename_r + 0
7  swift                    0x000000010d0a4929 swift::Lowering::CleanupManager::forwardCleanup(swift::DiverseStackBase::stable_iterator) + 313
8  swift                    0x000000010d0a77e4 swift::Lowering::ManagedValue::materialize(swift::Lowering::SILGenFunction&, swift::SILLocation) const + 116
9  swift                    0x000000010d0cca4c swift::Lowering::SILGenFunction::prepareAccessorBaseArg(swift::SILLocation, swift::Lowering::ManagedValue, swift::CanType, swift::SILDeclRef) + 1084
10 swift                    0x000000010d1576cc (anonymous namespace)::AccessorBasedComponent<swift::Lowering::LogicalPathComponent>::prepareAccessorArgs(swift::Lowering::SILGenFunction&, swift::SILLocation, swift::Lowering::ManagedValue, swift::SILDeclRef) + 172
11 swift                    0x000000010d1554c1 (anonymous namespace)::GetterSetterComponent::set(swift::Lowering::SILGenFunction&, swift::SILLocation, swift::Lowering::ArgumentSource&&, swift::Lowering::ManagedValue) && + 145
12 swift                    0x000000010d146677 swift::Lowering::LogicalPathComponent::writeback(swift::Lowering::SILGenFunction&, swift::SILLocation, swift::Lowering::ManagedValue, swift::Lowering::MaterializedLValue, bool) + 775
13 swift                    0x000000010d15699b (anonymous namespace)::GetterSetterComponent::writeback(swift::Lowering::SILGenFunction&, swift::SILLocation, swift::Lowering::ManagedValue, swift::Lowering::MaterializedLValue, bool) + 219
14 swift                    0x000000010d153bba swift::Lowering::ExclusiveBorrowFormalAccess::performWriteback(swift::Lowering::SILGenFunction&, bool) + 314
15 swift                    0x000000010d153d03 swift::Lowering::ExclusiveBorrowFormalAccess::finishImpl(swift::Lowering::SILGenFunction&) + 19
16 swift                    0x000000010d0a67dd swift::Lowering::FormalEvaluationScope::popImpl() + 269
17 swift                    0x000000010d0c258d swift::Lowering::SILGenFunction::emitApply(std::__1::unique_ptr<swift::Lowering::ResultPlan, std::__1::default_delete<swift::Lowering::ResultPlan> >&&, swift::Lowering::ArgumentScope&&, swift::SILLocation, swift::Lowering::ManagedValue, llvm::ArrayRef<swift::Substitution>, llvm::ArrayRef<swift::Lowering::ManagedValue>, swift::Lowering::CalleeTypeInfo const&, swift::Lowering::ApplyOptions, swift::Lowering::SGFContext) + 2429
18 swift                    0x000000010d0c58a5 (anonymous namespace)::CallEmission::apply(swift::Lowering::SGFContext) + 3925
19 swift                    0x000000010d0c4825 swift::Lowering::SILGenFunction::emitApplyExpr(swift::Expr*, swift::Lowering::SGFContext) + 1477
20 swift                    0x000000010d1296a3 swift::ASTVisitor<(anonymous namespace)::RValueEmitter, swift::Lowering::RValue, void, void, void, void, void, swift::Lowering::SGFContext>::visit(swift::Expr*, swift::Lowering::SGFContext) + 83
21 swift                    0x000000010d11f761 swift::Lowering::SILGenFunction::emitIgnoredExpr(swift::Expr*) + 1217
22 swift                    0x000000010d188aa3 swift::ASTVisitor<(anonymous namespace)::StmtEmitter, void, void, void, void, void, void>::visit(swift::Stmt*) + 8595
23 swift                    0x000000010d186905 swift::Lowering::SILGenFunction::emitStmt(swift::Stmt*) + 21
24 swift                    0x000000010d1411f6 swift::Lowering::SILGenFunction::emitFunction(swift::FuncDecl*) + 502
25 swift                    0x000000010d0bf7ce swift::Lowering::SILGenModule::emitFunction(swift::FuncDecl*)::$_1::operator()(swift::SILFunction*) const + 302
26 swift                    0x000000010d0b71f6 swift::Lowering::SILGenModule::emitFunction(swift::FuncDecl*) + 774
27 swift                    0x000000010d192075 (anonymous namespace)::SILGenType::visitFuncDecl(swift::FuncDecl*) + 21
28 swift                    0x000000010d1905c3 (anonymous namespace)::SILGenType::emitType() + 163
29 swift                    0x000000010d190519 swift::Lowering::SILGenModule::visitNominalTypeDecl(swift::NominalTypeDecl*) + 25
30 swift                    0x000000010d0bc27b swift::Lowering::SILGenModule::emitSourceFile(swift::SourceFile*, unsigned int) + 795
31 swift                    0x000000010d0bd018 swift::SILModule::constructSIL(swift::ModuleDecl*, swift::SILOptions&, swift::FileUnit*, llvm::Optional<unsigned int>, bool) + 344
32 swift                    0x000000010d0bd61f swift::performSILGeneration(swift::FileUnit&, swift::SILOptions&, llvm::Optional<unsigned int>) + 95
33 swift                    0x000000010c98b04f performCompile(swift::CompilerInstance&, swift::CompilerInvocation&, llvm::ArrayRef<char const*>, int&, swift::FrontendObserver*, swift::UnifiedStatsReporter*) + 8655
34 swift                    0x000000010c987e62 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 2946
35 swift                    0x000000010c9426e8 main + 1128
36 libdyld.dylib            0x00007fff61a3c085 start + 1
37 libdyld.dylib            0x0000000000000058 start + 2656845780
Stack dump:
0.	Program arguments: /Library/Developer/Toolchains/swift-4.2-CONVERGENCE.xctoolchain/usr/bin/swift -frontend -c /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/ChmodTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/ChownTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/CopyTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/CreateDeleteTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/FileBitsTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/FileModeTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/FilePermissionsTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/GlobTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/LinkTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/MoveTests.swift -primary-file /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/OpenTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/PathCollectionTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/PathTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/StatTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/TemporaryTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/UtilityTests.swift /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/XCTestManifests.swift -emit-module-path /Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Intermediates.noindex/TrailBlazer.build/Debug/TrailBlazerTests.build/Objects-normal/x86_64/OpenTests~partial.swiftmodule -emit-module-doc-path /Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Intermediates.noindex/TrailBlazer.build/Debug/TrailBlazerTests.build/Objects-normal/x86_64/OpenTests~partial.swiftdoc -serialize-diagnostics-path /Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Intermediates.noindex/TrailBlazer.build/Debug/TrailBlazerTests.build/Objects-normal/x86_64/OpenTests.dia -emit-dependencies-path /Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Intermediates.noindex/TrailBlazer.build/Debug/TrailBlazerTests.build/Objects-normal/x86_64/OpenTests.d -emit-reference-dependencies-path /Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Intermediates.noindex/TrailBlazer.build/Debug/TrailBlazerTests.build/Objects-normal/x86_64/OpenTests.swiftdeps -target x86_64-apple-macosx10.10 -enable-objc-interop -sdk /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -I /Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Products/Debug -F /Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Products/Debug -F /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/Library/Frameworks -g -module-cache-path /Users/jaketw47/Library/Developer/Xcode/DerivedData/ModuleCache.noindex -swift-version 4.2 -enforce-exclusivity=checked -Onone -D SWIFT_PACKAGE -D DEBUG -D Xcode -serialize-debugging-options -serialize-debugging-options -Xcc -fmodule-map-file=/Users/jaketw47/Documents/Programming/TrailBlazer/TrailBlazer.xcodeproj/GeneratedModuleMap/Cglob/module.modulemap -Xcc -fmodule-map-file=/Users/jaketw47/Documents/Programming/TrailBlazer/TrailBlazer.xcodeproj/GeneratedModuleMap/Cdirent/module.modulemap -Xcc -I/Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Intermediates.noindex/TrailBlazer.build/Debug/TrailBlazerTests.build/swift-overrides.hmap -Xcc -I/Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Products/Debug/include -Xcc -I/Users/jaketw47/Documents/Programming/TrailBlazer/.build/checkouts/Cglob--3738077931878635553/Sources/Cglob/include -Xcc -I/Users/jaketw47/Documents/Programming/TrailBlazer/.build/checkouts/Cdirent-5543099123802759031/Sources/Cdirent/include -Xcc -I/Users/jaketw47/Documents/Programming/TrailBlazer/TrailBlazer.xcodeproj/GeneratedModuleMap/Cglob -Xcc -I/Users/jaketw47/Documents/Programming/TrailBlazer/TrailBlazer.xcodeproj/GeneratedModuleMap/Cdirent -Xcc -I/Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Intermediates.noindex/TrailBlazer.build/Debug/TrailBlazerTests.build/DerivedSources/x86_64 -Xcc -I/Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Intermediates.noindex/TrailBlazer.build/Debug/TrailBlazerTests.build/DerivedSources -Xcc -DDEBUG=1 -Xcc -working-directory/Users/jaketw47/Documents/Programming/TrailBlazer -module-name TrailBlazerTests -o /Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Build/Intermediates.noindex/TrailBlazer.build/Debug/TrailBlazerTests.build/Objects-normal/x86_64/OpenTests.o -index-store-path /Users/jaketw47/Library/Developer/Xcode/DerivedData/TrailBlazer-ehyvhwwcrnifrzdrdhpruqpywpjq/Index/DataStore -index-system-modules 
1.	While emitting SIL for 'testSeek()' at /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/OpenTests.swift:311:5
2.	While silgen emitFunction SIL function "@$S16TrailBlazerTests04OpenC0C8testSeekyyF".
 for 'testSeek()' at /Users/jaketw47/Documents/Programming/TrailBlazer/Tests/TrailBlazerTests/OpenTests.swift:311:5
error: Abort trap: 6

This is only when building the tests though. I can build the project successfully.

OK, that looks like it could still be the same problem. I have a fix you may be able to try momentarily, once the "build toolchain" job completes: SILGen: Borrow the base of accessor LValueComponents. by jckarter · Pull Request #19902 · apple/swift · GitHub

1 Like