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

Unable to return a sending value as sending without an error #75473

Closed
mattmassicotte opened this issue Jul 25, 2024 · 11 comments
Closed

Unable to return a sending value as sending without an error #75473

mattmassicotte opened this issue Jul 25, 2024 · 11 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. concurrency Feature: umbrella label for concurrency language features

Comments

@mattmassicotte
Copy link
Contributor

Description

It seems like something about the use of self's state within the closure is defeating sending. I would have assumed that value here would always be possible to send.

Reproduction

public class Context {
	let value: Int

	init(value: Int) {
		self.value = value
	}
}

public struct Transaction {
	public static func with<T>(block: (inout Transaction) throws -> T) throws -> sending T {
		var txn = Transaction()

		return try block(&txn)
	}
}

actor MyActor {
	private var context = Context(value: 0)

	public func withContext<T>(_ block: sending (Context) throws -> T) async throws -> sending T {
		let value: T = try Transaction.with { txn in
			return try block(context)
		}

		// ERROR: returning 'self'-isolated 'value' as a 'sending' result risks causing data races
		return value
	}
}

Expected behavior

I think I understand why value is considered self-isolated. But, because it is marked sending, it really seems like it should be guaranteed possible to send, no?

Environment

Apple Swift version 6.0-dev (LLVM ec7116ccf9bd8a8, Swift e6aae02)
Target: arm64-apple-macosx14.0

Additional information

No response

@mattmassicotte mattmassicotte added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels labels Jul 25, 2024
@hborla hborla added concurrency Feature: umbrella label for concurrency language features and removed triage needed This issue needs more specific labels labels Jul 26, 2024
@gottesmm
Copy link
Contributor

I think that you are misunderstanding what is happening here. Part of this is that we do not have isolation history yet (which would explain exactly what is happening with value).

What is happening here is that Transaction.with is actor isolated I believe b/c it captures context (which is actor isolated). That then would cause value to be also actor isolated.

Btw a tip for you that might be helpful... you can dump the silgen using -emit-silgen to figure out quickly what the isolation is of specific functions... we print out the inferred isolation of functions. As an example using your test case:

// closure #1 in MyActor.withContext<A>(_:)                                                                               
// Isolation: actor_instance. name: 'self'                                                                                
sil private [ossa] @$s5test27MyActorC11withContextyxxAA0E0CKXEYaKlFxAA11TransactionVzKXEfU_ : $@convention(thin) <T> (@inout Transaction, @guaranteed @noescape @callee_guaranteed @substituted <τ_0_0> (@guaranteed Context) -> (@out τ_0_0, @error any Error) for <T>, @sil_isolated @guaranteed MyActor) -> (@out T, @error any Error) {                                 

@mattmassicotte
Copy link
Contributor Author

mattmassicotte commented Aug 1, 2024

Yeah, that isolation history thing sounds amazing. And I totally believe I could be confused! But here's what I do not get. Doesn't a sending return value mean a function must produce a value that is possible to send? And if not, then what does it mean?

If there was an error in Transaction.with, it would make more sense to me I think...

@gottesmm
Copy link
Contributor

@mattmassicotte so I just took another look at this. I actually misread your post. I thought you were referring to the sending on withContext, not the sending on with. My apologies. I think there is an issue here.

@mattmassicotte
Copy link
Contributor Author

No problem at all! I've also since realized that the signature of with doesn't make sense. I intended it to be this, but I think I just got derailed.

public static func with<T>(block: (inout Transaction) throws -> sending T) throws -> sending T

And while that affects the overall correctness of the code example, I think this issue still stands. Plus, that particular version currently crashes as noted in #75982

@gottesmm
Copy link
Contributor

#76076

@mattmassicotte
Copy link
Contributor Author

Wow, this is amazing! Thank you so much for looking at this again.

@gottesmm
Copy link
Contributor

@mattmassicotte can you verify with the next snapshot that comes out? Then I think we can close this out? Or we can wait until this hits Xcode (eventually) and close it out then... your choice.

@mattmassicotte
Copy link
Contributor Author

I check it out as soon as a new snapshot is released!

@mattmassicotte
Copy link
Contributor Author

Just tested this out with the Sept 11 snapshot, but no change. I assume that this change just wasn't picked up yet.

@mattmassicotte
Copy link
Contributor Author

@gottesmm do you think this is something that might make it into the 6.1 release?

@mattmassicotte
Copy link
Contributor Author

Verified with 6.1 snapshot. Thank you, this rocks!

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. concurrency Feature: umbrella label for concurrency language features
Projects
None yet
Development

No branches or pull requests

3 participants