[WIP]: inherit caller isolation in some AsyncChannel methods

[jamieQ]

This change adopts caller isolation inheritance via parameter isolation in Async{Throwing}Channel and corresponding AsyncIterator types.

Resolves: #356

[jamieQ]

cc @FranzBusch @phausler – this is largely a PoC of how we might address some executor hops that could lead to unexpected event orderings. i'd like to hear your thoughts on the approach. in particular:

1. are there concerns with adding defaulted isolated parameters into existing API?
2. is tackling this sort of thing in a piecemeal fashion palatable, or have you considered a wholesale audit of the repo to address the class of issue?
   • relatedly, do you have thoughts regarding opting into NonisolatedNonsendingByDefault at some point (though i'm personally not sure it's totally ready for all use cases yet)?

[FranzBusch]

Thanks for opening this. I think this is a good effort and we should inherit isolation in all async methods possible both for correctness and performance. I do think we should try to do this by enabling NonisolatedNonsendingByDefault. We have it enabled in a few repos already and it seems to work well. It also makes it easier to adopt since it will just flip the default for 6.2+ and stay the same on older compiler versions. Did you give it a try to compile this repo with that default enabled and see if we run into any issues?

[jamieQ]

Did you give it a try to compile this repo with that default enabled and see if we run into any issues?

@FranzBusch i did a very brief test, and there were many errors involving non-sendability that immediately surfaced for isolation boundary closures (i believe possibly fixed in swiftlang/swift#85603 and friends). also, basically every async iterator next() implementation errors b/c the existing implementations use the non-isolation-inheriting form.

IIUC just enabling that feature now also would not resolve the issues with hops when calling the concurrency runtime functions (with*(...)) until they are somehow changed. it's not clear to me if any of the in-flight PRs to do that will ensure the semantic change is realized on older runtimes though.

[FranzBusch]

Thanks for the investigation. I expected that we will need a multi-step effort to get this repo to compile cleanly with the feature enabled. We should do it nevertheless.

IIUC just enabling that feature now also would not resolve the issues with hops when calling the concurrency runtime functions (with*(...)) until they are somehow changed. it's not clear to me if any of the in-flight PRs to do that will ensure the semantic change is realized on older runtimes though.

My recent testing showed that in 5.10 and below the Concurrency runtime with methods were behaving correctly and that this is a regression since 6.0. I don't expect that older runtimes are fixed retroactively.

[jamieQ]

@FranzBusch thanks, that makes sense. i'll close this then if the piecemeal strategy isn't appropriate.

I don't expect that older runtimes are fixed retroactively.

just to clarify – i also don't expect this, but my thinking is just that today there is a way to address certain classes of unexpected executor hops with the existing runtime functions that take isolated parameters: ensure the caller has an isolated parameter too, and that it is captured by the closures passed to the runtime functions. but that technique appears to not work if an explicit isolated parameter function is converted to nonisolated(nonsending) – or at least, it doesn't work without some awkward boilerplate i think. so, the concern would be that these issues could in theory be addressed on the 6.0-6.2 runtimes via explicit isolation inheriting functions, but if we 'just' convert to NonisolatedNonsendingByDefault then those runtimes can still exhibit these issues (i think).

[jamieQ]

closing in favor of some more holistic approach