-
Notifications
You must be signed in to change notification settings - Fork 4
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
Is implicit suspension really such a good idea? #10
Comments
I like the opt in/out idea. |
The idea with the current proposal is already opt-in explicitness implemented through a normal suspending function called E.g. import PromiseAwait.await;
function makeRequest():Promise;
suspend function someFunction() {
var data = await(makeRequest()); // or makeRequest().await() with `using`
} Isn't that explicit enough? |
Ok, let me rephrase that: what options does a 20+ person team have to make sure everything is explicit within a specific critical part of the code? Imagine this: suspend function someFunction() {
var foo = getFoo();
var bar = await(getBar());
...
} Everybody reading this will think that |
So this basically comes down to a whitelist of suspending functions that are allowed to be directly called. I'm pretty sure this can be implemented on top of this proposal, even with a macro (but of course it would be faster if implemented in compiler). Let's leave this open so we don't forget to experiment with it. |
Hmm, perhaps I'm misunderstanding. Will suspension only be implicit within functions marked with |
That sounds good to me. Any |
After giving this more thought, I would like to ask for an option for disabling implicit suspension after all. In theory, this feature seems like a good fit for asys (or just nodejs). So much so, that it's conceivable that one would want to build an entire server with it (and indeed it's not uncommon to see nodejs servers built almost entirely on In such a scenario, I think it's important to be able to enforce a coding style where suspension must be explicit, because while each suspension-free section is atomic, every suspension yields control, thereby opening the door for race conditions. This isn't much of a problem for dialogs/transitions, but on a server handling thousands of concurrent users, one careless suspension can lead to bugs. The type of bugs that only occur in production under load. In a code review, it would therefore be nice to have them stand out. And the possibility of silently introducing a new implicit suspension into the codebase merely by updating a dependency only to find out in production is also one less thing I'd like to worry about. |
Note that the "stand out" part doesn't necessarily mean that it has to be written in the code. As long as the compiler, and thus the IDE, know what suspends and what doesn't, this information can be shown to the user in some way. I guess we could provide the option to force explicit suspension, but in that case it should be package-based (like null-safety) so one can still use dependencies that don't adhere to this limitation. |
In Kotlin there's a feature called restricted suspension that kind of provides something like that - it forbids calling any suspend functions that are not methods of the given "receiver" (another kotlin feature, which is basically configurable We can think of some metadata that could provide such restriction in different scopes (package-level, class-level, method-level, expression-level), then it would be possible to use this for both code style enforcement and scenarios like generator+yield. (this might be actually already be possible with a macro) |
The reason I'm kind of worried about this is that I can see myself using it on a nodejs server to write request handlers that are run with high concurrency.
The thing I like about explicit use about a callback, promise or
await
is that looking at the code I can see that execution is suspended. I think it's pretty important to see these places, because it's exactly the ones where race conditions may occur in an execution model such as node's (or that of a browser). On a large enough project the implicitness may create more problems than it solves. If for some reason (e.g. a large enough team) any of the functions called from asuspend function
becomes suspendable itself, the change passes silently, even though it potentially introduced a bug that only surfaces under high concurrency and therefore in all likelihood won't be caught by tests either and instead occasionally occurs in production - the kind of bug that nobody wants to have to deal with.I can see the appeal of the implicitness and in many scenarios I think I would use it myself (e.g. in functional code or in small scripts). So I wonder if we could somehow make this an opt-in/opt-out. Perhaps something like
using ImplicitSuspend;
/using ExplicitSuspend;
would be good, because then you can make it per module or even per folder withimport.hx
.The text was updated successfully, but these errors were encountered: