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

Production hang updating from 1.1.5 to 1.2.x #199

Closed
2 of 3 tasks
finestructure opened this issue Mar 28, 2024 · 3 comments
Closed
2 of 3 tasks

Production hang updating from 1.1.5 to 1.2.x #199

finestructure opened this issue Mar 28, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@finestructure
Copy link
Contributor

Description

I'll admit this is a very weird one and I'm not sure where to start to debug it but it's a reproducible production affecting issue.

In our SPI build system we build a "builder" binary using Xcode 15.3 (Swift 5.10) and distribute it to Macs that run it to build packages with various Swift versions and platforms. We have a integration test matrix set up to run a package build for all combinations.

When I do a package update for our builder, updating swift-dependencies from 1.1.5 to 1.2.x (or main) causes a hang in the builder process. I.e. our package builds time out after 10min. This hang is only happening on macOS, Linux is working fine.

Our (closed source) builder doesn't use @Dependency itself but transitively via the open source package DocUploader.

I can tell from the logs that it's hitting @Dependency(\.uuid) var uuid. And indeed if I replace this with let uuid = { UUID() } the hang disappears. (There's a second @Dependency in the project but our test doesn't cover it, so it's unknown if it would hang, too.)

Is there anything in the 1.1.5...1.2.0 diff that could cause a hang like this on macOS?

I see a couple of #if _runtime(_ObjC) blocks that do things with DispatchQueue that seem likely candidates but I can't tell how they're used or if they could in fact be the cause.

FWIW, we have a unit test in our builder which should be doing the same as the integration test is not hanging. I've also tried running the builder with a -c debug build and it's still hanging. So it's happening in both debug and release builds but not when the dependency is being replaced via withDependencies.

I'm not sure how I could debug this further but I do have a reproducer set up that can turn around results in ~5mins. I'll see if I can boil this down into a reproducer that's less complicated to run that I can share.

For now I'm working around the issue via

#if DEBUG
    @Dependency(\.uuid) var uuid
#else
    let uuid = { UUID() }
#endif

which is of course not ideal but I at least I don't need to pin swift-dependencies to 1.1.5.

Checklist

  • I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
  • If possible, I've reproduced the issue using the main branch of this package.
  • This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

It should not hang 😄

Actual behavior

It hangs 😞

Steps to reproduce

Currently only internally reproducible, working on a sharable project.

Dependencies version information

1.2.x

Destination operating system

macOS 14

Xcode version information

15.3

Swift Compiler version information

swift-driver version: 1.90.11.1 Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
@finestructure
Copy link
Contributor Author

Ok, I've found the issue via a stand-alone reproducer: https://github.com/finestructure/dependencies-hang-repro

The problem was that we still used an "old style" entrypoint to run an async main function from a main.swift file:

import Foundation

let group = DispatchGroup()
group.enter()

Task {
    defer { group.leave() }
    await Builder.main()
}

group.wait()

Once I change this to

@main
struct Executable {
    static func main() async throws {
        await Builder.main()
    }
}

the hang disappears.

This is certainly our bug (I'm unsure if the DispatchGroup thing was ever ok but it's certainly been working across millions of builds so far 😅). Might still be helpful info in case others have been doing this as well. I can't remember if I came up with this thing or if I got it from somewhere else!

@mbrandonw
Copy link
Member

Hi @finestructure, it's most likely related to this PR #169. In general, because we need to do threading shenanigans in Dependency.init in order to register with the test observer, it's just not a good idea to do any locking shenanigans yourself upon first access of an @Dependency.

However, curious why spin up the Task at all and not just do the await Builder.main() in the root of main.swift? It supports async contexts, and that also fixes the problem. And had you done the same locking behavior in the @main version you get the same problem:

@main
struct Executable {
  static func main() throws {
    let group = DispatchGroup()
    group.enter()

    Task {
        defer { group.leave() }
        await Builder.main()
    }

    group.wait()
  }
}

@finestructure
Copy link
Contributor Author

This main file was from before it supported top level async code! That’s how we ended up with that DispatchGroup :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants