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

waitFor is still experimental after two years? #39390

Closed
timsneath opened this issue Nov 15, 2019 · 50 comments
Closed

waitFor is still experimental after two years? #39390

timsneath opened this issue Nov 15, 2019 · 50 comments
Assignees
Labels
area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …). docs-api

Comments

@timsneath
Copy link
Contributor

The waitFor function has been marked as "experimental" for two years now:

* WARNING: EXPERIMENTAL. USE AT YOUR OWN RISK.

(see https://dart-review.googlesource.com/c/sdk/+/28920/ for its introduction)

Can we remove this line at this point? The experiment has clearly run its course. Either we need to remove this comment or we need to deprecate it (if we think it's a failed experiment for some reason).

There is value in something like this for CLI apps, of which there will be many more now that dart2native has landed, and I'd love us to provide reliable guidance. If there are better alternatives towaitFor, I'd love to know about them myself!

@timsneath timsneath added docs-api area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …). labels Nov 15, 2019
@rmacnak-google
Copy link
Contributor

This function should be removed. It is fundamentally incompatible with Dart's event loop model.

@timsneath
Copy link
Contributor Author

timsneath commented Nov 15, 2019

OK. If that really is the case, then can we get it marked as @deprecated so we can start the countdown?

Leaving it around causes confusion, as evidenced by the fact that I just spent time introducing it into a CLI app I was writing. The code is cleaner than the highly-contagious async / await that I had before, but if it's not good code then I'd prefer to be forewarned...

@bkonyi
Copy link
Contributor

bkonyi commented Nov 27, 2019

If I recall correctly this was being used by @nex3 in SASS.

@nex3
Copy link
Member

nex3 commented Nov 27, 2019

Yes, we rely heavily on waitFor()—there's no other way in Dart to call out to asynchronous APIs from otherwise-synchronous code. Please don't remove it.

@timsneath
Copy link
Contributor Author

Is the alternative to use then()?

@rmacnak-google, how do we resolve this?

@mraleph
Copy link
Member

mraleph commented Dec 2, 2019

Using then() is not an alternative. You want interface that looks synchronous but performs asynchronous actions inside. This is indeed in contradiction with Dart's event model as @rmacnak-google says and this function probably should not have been added in the first place.

Currently the library has been hidden from the public documentation by @mit-mit

I don't think we currently can resolve this further at the moment: in general a lot of engineers are in favour of removing it, but I don't think we can given that dart-sass depends on it.

@nex3 how does the code that depends on waitFor work on node? I recall that you could run dart-sass in two modes: native and translated to JS. Is this still the case?

@nex3
Copy link
Member

nex3 commented Dec 17, 2019

Describing waitFor() as "in contradiction with Dart's event model" is too strong—plenty of languages support an event-loop based model while also being compatible with synchronous calls. This is the basis of the concept of fibers, for example, and very similar to how Go's goroutines work. waitFor() is just an implementation of the special case of that concept with a single stack shared between all events.

For what it's worth, I strongly believe full fiber support on the VM would be a substantial benefit to Dart.

@nex3 how does the code that depends on waitFor work on node? I recall that you could run dart-sass in two modes: native and translated to JS. Is this still the case?

In JS mode, we currently use the fibers package. We're exploring the possibility of switching to worker threads along with the Atomics.wait() function, although whether we do so in practice depends on how high the overhead of spawning a thread turns out to be.

@bsutton
Copy link

bsutton commented Jan 14, 2020

I would like to add my voice to keeping waitFor and removing the experimental tag.

I've just implemented a package call dshell which provides tooling and a library for building cli applications.

https://pub.dev/packages/dshell

dshell makes heavy use of waitFor (as in virtually every exposed function uses it).

The use of waitFor greatly simplifies the dshell users experience and actually makes the library safer to use.
During our early experiementation for dshell we played with an async version of the library.
We found it was very easy to miss a single 'await' and the whole script would start mis-behaving in unexpected and hard to debug ways.
As an example you might have started an async write to a file, forget to await the write and so end up trying to move a the file before the write completes.
We were spending time scanning through our code to ensure that every method up the stack was properly awaited.

There is also little gain in providing an async experience when writing cli scripts as there is no user expectation that the UI should be responsive when long running tasks are running.

Removing waitFor would be determintal for writers of cli applications and would completely kill dshell.

So I would much rather it waitFor remains :)

@knopp
Copy link
Contributor

knopp commented Jan 14, 2020

Most run loops I've used can be run recursively, in order to handle situations like this. libuv is the only one I can think of that can't. CFRunLoop, Qt event loop and GMainLoop all are reentrant in order to allow synchronous callbacks. Since in dart we have no control over the loop itself, waitFor is as good as it gets for an escape hatch.

@natebosch
Copy link
Member

We might want to look at alternative APIs that involve Isolates. I think it would be less of a violation of Dart semantics to let you run async code in a separate isolate and synchronously wait for the result.

@nex3
Copy link
Member

nex3 commented Jan 12, 2021

Isolates are slow to spawn and expensive to send information across. If they're not being used for actual parallelism, insisting that they be used for synchronicity is just adding end-user pain for no value.

@bsutton
Copy link

bsutton commented Jan 12, 2021 via email

@leafpetersen
Copy link
Member

Describing waitFor() as "in contradiction with Dart's event model" is too strong—plenty of languages support an event-loop based model while also being compatible with synchronous calls.

Is there a short summary of what the current "event model" is that this feature violates? I'm guessing it may have to do with things escaping the scope of a nested loop? Thinking about this at a high level, Haskell has runST precisely to handle this kind of scenario (safely escaping a monad) but they use parametric polymorphism to ensure that monadic values don't "escape". Does that get at the issue, or is it unrelated?

@keertip
Copy link
Contributor

keertip commented Feb 18, 2021

Just as an FYI, another use case for waitFor() is in the language server for Cider to turn asynchronous cross isolate communication to synchronous. We have multiple worker isolates and a common cache isolate.

@lrhn
Copy link
Member

lrhn commented Feb 19, 2021

The "violation" is synchronous code being interrupted by asynchronous code.
Example:

var sub = stream.listen(null);
callSomeMethod();
sub.onData((v) { action(); });
sub.onError((v) { handle(v); });

We generally promise that no events are sent on a subscription returned by listen until a later microtask/event, so when the current synchronous code has completed. That allows you to add event listeners later.

If callSomeMethod() ends up running waitFor, then we'll run a full event loop for a while, which might trigger events on sub before we have added the event listeners. If that's an error, it might become an uncaught async error and take down your entire application. If it's data, you might miss the data.

Similarly for futures:

var future = compute():
var value = callSomeMethod();
return future.catchError((e, s) => value);

Here the error on future might happen before you add the catchError.

In short: Apparently synchronous code isn't actually synchronous.

(This is not unique to waitFor, a misuse of a synchronous completer or stream controller can cause the same problems, but that actually requires code to be misbehaving. Here we can make perfectly good code fail just by calling waitFor, which suggests that waitFor is the misbehavior.)

We also break await for loops:

bool inLoop = false;
await for (var e in stream) {
  assert(!inLoop);
  inLoop = true;
  callSomeMethod();
  inLoop = false;
}

The specification of await for loops require the subscription of the loop to be paused when the body does something asynchronous. It doesn't pause it unconditionally when entering the body because that would be an unnecessary overhead when the entire body is synchronous.
If we do a waitFor inside callSomeMethod, we'll keep the event loop running and can potentially re-enter the body on the next data event. There is no way to know that we should have paused the loop subscription, the waitFor call is not visible from here, and the loop isn't visible in the other direction.
(I guess an implementation could remember the "current await-for loop subscription" for synchronous computations, and pause it, if there is one, when calling waitFor. I don't think that's happening now).

All in all, waitFor is breaking the invariant that nothing async will happen before synchronous code has finished. We have based a lot of APIs and some language feature design on that invariant.
(And again, a completely misbehaving stream, say one which ignores both pause and cancel and keeps firing events, can likely make an await for loop also act unpredictably, but waitFor can break good code).

@nex3
Copy link
Member

nex3 commented Feb 19, 2021

All in all, waitFor is breaking the invariant that nothing async will happen before synchronous code has finished.

This is only an invariant by fiat, though, and in practice it's quite a troublesome one. In Dart Sass, we need waitFor() precisely because we want to call user-defined callbacks from a very large chunk of otherwise-synchronous code that is also invoked in synchronous contexts.

There are plenty of existence proofs of languages that work fine without providing this invariant. Fibers in Ruby and the fibers package for Node.js both make it possible to run the event loop from within a synchronous context, and they don't break the world. Nor has waitFor() broken it, for that matter.

It sounds like the biggest risk with waitFor() is that it could be called unexpectedly from a synchronous function whose implementation you don't understand and wreak havoc by mutating state unexpectedly or something like that. But that's already true—if you don't understand a function's implementation, it could do just about anything even without calling waitFor(). And it's doubly true in an async world with awaits sprinkled everywhere, and it doesn't seem to cause that much pain there.

@bsutton
Copy link

bsutton commented Feb 19, 2021 via email

@leafpetersen
Copy link
Member

If callSomeMethod() ends up running waitFor, then we'll run a full event loop for a while, which might trigger events on sub before we have added the event listeners. If that's an error, it might become an uncaught async error and take down your entire application. If it's data, you might miss the data.

Sorry for the basic questions, but I'm really only passingly familiar with the implementation here. Am I correct in thinking that "trigger events on sub" can only happen if some code running in the nested event loop accesses sub directly through some shared mutable state? That is, my naive expectation is that running a nested event loop essentially stops processing the existing event loop and starts running a new event loop. If you don't put anything from the outer event loop onto the inner event loop then.... nothing happens as far as the outer event loop is concerned. It's only if you let asynchronous data structures "leak" across event loop domains that you get into trouble.

Is that correct? If not, what is incorrect about my (naive, high level) mental model above?

If it is correct, it seems like one approach is just to say "touching asynchronous computations from a different event loop is undefined badness, don't do it"? You could even imagine trying to enforce this at runtime by attaching event loop identifiers?

@leafpetersen
Copy link
Member

@bsutton Thanks very much for the input here, it's very valuable to know that people are using the feature and what they are using it for.

@lrhn
Copy link
Member

lrhn commented Feb 22, 2021

@leafpetersen

Am I correct in thinking that "trigger events on sub" can only happen if some code running in the nested event loop accesses sub directly through some shared mutable state?

No, no access to the subscription is needed, unless you could the reference to the subscription held by the underlying stream controller. Then it's "yes", but also always the case.
If anything on the event loop (which is pretty much the entire rest of the program, including all scheduled microtasks, timers and I/O, or any later port events) causes events on the stream to happen, then those events can be lost. That's what a call to listen usually ensures will happen.

Consider:

var stream = someFile.openRead();
var sub = stream.listen(null);
someMethod(); // ends up calling waitFor
sub.onData(handleBytes);

This call to listen starts an I/O operation reading the file. At some later point, the data events are added as events (probably port events) to the main event loop, and propagates them to the stream subscription. Since the code in someMethod is running the full event loop, those events all get delivered, and the null handler initially attached to sub throws the data away. Maybe there'll be a done event too.
Then the future waited on by waitFor completes, and the nested event loop returns, and then we finally get to add the event handlers to the subscription. We may get a partial file or no events at all, not even a done event.

@nex3
The invariants I speak about might only be by fiat of the implementation trying its darndest to ensure them, but it is also something we have repeatedly told people that they can rely on. If those invariants break, be it because someone uses a sync completer or controller incorrectly or because they use waitFor incorrectly, we should blame the incorrect use.
We have specified where you can safely use the synchronous completers/controllers, we haven't said that for waitFor (it's probably the same places: Inside another asynchronous event, only surrounded by code which is aware that arbitrary things might happen when you call it).

In that sense, waitFor is not that different from synchronous completers or controllers. It's probably more dangerous (it affects all code which assumes that no async events run during sync code, not just events from a particular future or stream), but if you use it only correctly, things shouldn't break.

Another worry I have is that it might not scale. Since the waitFor calls are stack-ordered, the latest call blocks all prior calls from continuing, even if their futures are completed. That means that it's possible to introduce a deadlock if the latest future depends on the completion of an earlier waitFor'ed future. If only one package in the program uses waitFor, it can probably maintain a strategy which avoids problems, but if you have two or more packages or frameworks where all use waitFor, then I think the risk gets higher. That makes it a dangerous feature to depend on. (But a different approach, with multiple stacks, could alleviate that).

@nex3
Copy link
Member

nex3 commented Feb 22, 2021

@lrhn What I'm trying to say is that the actual concrete effects of running async events during sync code aren't intrinsically any more dire than the effects of running unknown synchronous code. This isn't just because of sync controllers/completers, it's because the Dart code that's run in async callbacks could in theory just as easily be run synchronously through the normal call stack—state could still be manipulated in unexpected ways by a badly-behaved program. There's no guarantee that synchronous code only has local, well-documented effects.

(But a different approach, with multiple stacks, could alleviate that).

I would certainly be in favor of moving to Ruby-style multistack Fibers 😃.

@knopp
Copy link
Contributor

knopp commented Feb 22, 2021

Consider:

var stream = someFile.openRead();
var sub = stream.listen(null);
someMethod(); // ends up calling waitFor
sub.onData(handleBytes);

Is that really any different from someMethod() calling CFRunLoopRunInMode or a nested message loop in windows? Perhaps bit less obvious, but that enough to warrant removal, given that there are legitimate use cases for it?

@jamesderlin
Copy link
Contributor

How about just maintaining the status quo? Keep waitFor, but to limit abusing it, don't advertise it, don't recommend it, and don't support it.

@bsutton
Copy link

bsutton commented Feb 22, 2021 via email

@bsutton
Copy link

bsutton commented Feb 23, 2021

Can put a slightly different perspective on this discussion.

waitfor is intended only for cli applications.

I don't think it's too big a stretch for me to suggest that I'm probably the most experienced end user of waitfor in the community through my work on the dcli library.

Dcli is a fairly large library that exposes a large no. of functions.
Virtually every one of these functions use waitfor.

I've been using dcli (and waitfor) for going towards 2 years to build large no.s of console apps both for our private usage and open source projects I've released.

Its perhaps a reflection of my own philosophy but my experience with building console app is that I almost never use async programming models. Async programing creates significant problems when writing console apps for no benefit.

Over all of the projects I've worked on I can only remember having one problem with waitfor which was I managed to cause a deadlock. The problem was fairly easy to identify and solve.
I've never seen any data lose on a stream (and I do use a couple internally within dcli).

My point is that I think people are a little over concerned with the possible interactions between waitfor and async code.

In my experience building console apps there is really little need to use async. In the real world the problems this thread is discussing are virtually non existent.

The possible risk (which I don't believe has ever been reported) is far outweighed by the benefits.

@leafpetersen
Copy link
Member

No, no access to the subscription is needed, unless you could the reference to the subscription held by the underlying stream controller. Then it's "yes", but also always the case.

Sorry, I really couldn't parse this. :)

This call to listen starts an I/O operation reading the file. At some later point, the data events are added as events (probably port events) to the main event loop, and propagates them to the stream subscription. Since the code in someMethod is running the full event loop, those events all get delivered, and the null handler initially attached to sub throws the data away. Maybe there'll be a done event too.

I think maybe this gets implicitly at what I'm missing. My mental model of "running a nested event loop" is that you would run with a new event and microtask queue. You seem to be assuming that this is not done (presumably because it is not in fact done in the current implementation). Is that right? So the shared mutable state is in fact the shared event loop itself?

@lrhn
Copy link
Member

lrhn commented Feb 24, 2021

@leafpetersen
Correct. The waitFor function is running the event loop code again, but using the same underlying event queues. If it didn't, then every async computation which had already been started would stall, and it's unlikely that the future you're waiting on would ever complete.

(And makes sense you couldn't parse the sentence, some words were typoe'd.)

@mraleph
Copy link
Member

mraleph commented Feb 25, 2021

@lrhn what is your take on Fibers? it allows to write synchronously looking code which can block and wait for events. It has some issues with reentrancy (e.g. an arbitrary piece of synchronous code might not be written to be "suspendable"), but otherwise it looks like a much more coherent approach than waitFor. There are languages like Lua (coroutines) and Ruby (Fibers) which have it builtin and they seem to fair fine.

@lrhn
Copy link
Member

lrhn commented Feb 25, 2021

If "fibers" are non-concurrent operations with separate stacks, which allows each fiber to block independently (and then return to the one and only event loop), then it's power is exactly the same as waitFor, just without the forced stack discipline and the (increased) risk of deadlock is gone. (The risks of misuse are exactly the same, if you block at a point where other code doesn't expect async events to happen, you can break that code).
So, fibers would be a good way to implement waitFor — and a great way to implement await, you wouldn't have to CPS-transform your async functions, just block the fiber and release it when that future is completed by some other fiber. And if an await is recognizable as await anAsyncFunction(), you can just keep running in the same fiber.

If we had fibers from the start, on all platforms, it would be great. We wouldn't have streams or futures then, they're implementation artifacts of not being able to do proper blocking operations. People writing code would know that any function call can cause arbitrary things to happen (so we'd probably want to have mutexes to prevent that). Much nicer. Alas, JavaScript won't allow that, so we still need Future and Stream.

@leafpetersen
Copy link
Member

I'm still not sure "a fresh event and microtask queue" is even well-defined.
The operation could very easily return a future created earlier instead of being a completely separate asynchronous computation where all futures are fresh.

Right. This is why I brought up shared state. I think Haskell avoids this via parametric polymorphism - basically runSt introduces a new "phantom type variable" which prevents mixing of state from different invocations. This isn't really feasible in Dart (we can't go index all of our asynchronous operations by a type now), so we would have to rely on some combination of convention/runtime checking.

If a port event comes in during the waitFor, which queue would it go into? The one which was current when the receive port was created? that would delay the port events that go into the old queue (and risk time-outs at a higher protocol level)?

Yes. The whole point is that you are now blocking right? If you block for too long... the outside world gets sad.

We can pause all timers while the old event queue is paused (that would be hugely surprising to code which expects to be run),

As best I understand, there are no guarantees whatsoever that code which is run with a timer will be run immediately after the timer expires, right? It can be blocked arbitrarily long already.

Then we also have issues if the new code spawns async operations which continue after the future it returned has completed (and we would tear down the new event queues). Do we have to wait for the fresh queue to be empty (which could be never), or do we merge it back on the older queue when the waitForCall completes?

I don't know, you tell me? I think I would expect that we run until the queue is empty. Which could indeed be never.

@lrhn
Copy link
Member

lrhn commented Feb 26, 2021

If the model would be "freeze the old world, run completely separate new code until it's done", then that would be hard to make work in Dart. You couldn't have any interaction with the "old world" of asynchrony, and because we have global state, that's unenforceable. That's why I'd prefer running the new code in a completely new isolate (even though it could run in the same thread as the original while that's blocked, and maybe even share the same heap, making sendAndExit quite efficient).

That would be something like Isolate.spawnAndWait<T, R>(FutureOr<R> Function(T) function, T argument) which runs function in a new isolate, and when that isolate dies, it returns the result of calling function, awaited if it's a Future<R> (or an AsyncError if the other side threw). I could implement that using a blocking port.

There'd still be the risk of the other isolate never terminating, even after the entry function has returned and completed, because of some forgotten asynchronous loop.

@bsutton
Copy link

bsutton commented Feb 26, 2021

@lrhn I don't see how spawnAndWait would be equivalent functionality.

As I understanding an isolate runs in its own memory with no shared state. If that is correct then your proposal would break existing code and would essentially make most functions that I currently use waitFor for untenable as they often rely on shared state.

I would also be concerned of the performance overhead. I've done no testing but I would assume that spawning an isolate is an expensive operation. I assume that dart has some sort of isolate worker pool but I assume the memory has to be re-initialised each time.
The other questions is whether it solves the problem. Wouldn't the spawnAndWait still need to suspend/prevert the current event loop just as waitFoEx does? It's still waiting in the main isolate.

@lrhn
Copy link
Member

lrhn commented Feb 26, 2021

Yes, spawnAndWait is not equivalent to the current waitFor, but to the hypothetical function (I think that) Leaf suggested which stopped all existing async behavior and ran something separate in a new, fresh, event loop. That code would not be able to touch any existing futures, because they probably won't complete until the new event loop ends, so having to move the execution to a different isolate would not be a big difference. It would require more copying (possibly, unmodifiable values could probably be shared), but it would have a cleaner execution model.

@blaugold
Copy link
Contributor

Another use case for the functionality that waitFor provides are synchronous callbacks from the native side when using ffi. When an isolate calls a native function that takes a callback and this function only calls that callback synchronously, we can pass it a function pointer of a static dart function. If the dart function wants to call async code it needs something like watiFor.

@mit-mit
Copy link
Member

mit-mit commented Sep 28, 2021

@mit-mit mit-mit assigned mit-mit and unassigned lrhn Sep 28, 2021
@bsutton
Copy link

bsutton commented Sep 28, 2021

@mit-mit

I've just seen that waitFor had been marked as depreciate.

I strongly protest this decision.

waitFor is a critical function for the dcli package and general cli programming.

Please reverse this unnecessary decision.

No one in this thread has actually demonstrated that waitFor is broken in real world code and having used it everyday for the past two years I can tell you it works just fine.

This decision is overly proscriptive and does nothing to improve the dart ecosystem and demonstratively reduces darts functionality.
If you have something to replace it with then let's discuss it, until then please leave waitFor alone.

@natebosch
Copy link
Member

If you have something to replace it with then let's discuss it, until then please leave waitFor alone.

We don't have immediate plans to delete the existing implementation. We are deprecating it primarily to discourage new uses and to align the documentation with the the level of support we intend to provide for it.

@bsutton
Copy link

bsutton commented Sep 28, 2021 via email

@mraleph
Copy link
Member

mraleph commented Sep 28, 2021

This is an API which was always explicitly marked experimental and thus can be removed at any time with no replacement. Some of us are strongly in favor of doing that. The only reason why we have not is because this API currently is not a strong maintenance burden and it has a very small number of passionate users who made an unfortunate mistake of relying on an experimental API. So we weighted the cost of maintenance vs the cost of breaking these few users and decided that breaking is not worth it at this point in time. That being said this balance can shift any moment and this API can be deleted - so you are encouraged to migrate away at your earliest convenience.

@nex3
Copy link
Member

nex3 commented Sep 28, 2021

It's unfair to characterize users as "making a mistake" for using this API, whether or not it's marked as experimental. It's the only available avenue to address a substantial missing piece of functionality in Dart: the ability to write code that's polymorphic over synchrony versus asynchrony. This is clearly functionality that users need, and shaming them for taking the only option for making that happen isn't only useless, it's actively hostile to the people who use your product. If you want to ensure that people don't use this API, offer a better alternative, don't talk down to your users.

@bsutton
Copy link

bsutton commented Sep 28, 2021 via email

@natebosch
Copy link
Member

natebosch commented Sep 28, 2021

The typical user of waitfor is also unlikely to be even aware of this issue.

This informs our decision of marking it deprecated. The existing mechanism we had clearly wasn't strong enough, and marking it as deprecated should make it more obvious. We don't want the typical user to remain unaware of this issue.

This whole conversation seems to be driven by people that aren't using the API, pontificating on how bad it is, whilst the actual users are saying this is critical and it works.

This conversation is driven by the people who would be on the hook for fixing this API if we make it officially supported and it breaks. We've decided we don't want to make those promises because it carries risk and may limit our ability to change implementation details.

The framing for the deprecation is not "The dart team changed their mind and decided to stop supporting this." The framing of the deprecation is "The dart team is fixing their mistake of having an unsupported API which doesn't clearly advertise the lack of support."

Any discussion about the history is not intended to assign blame or make anyone feel bad about their decisions. The discussion of the history is explain that the unclear advertising of the support level is not a deciding factor that will force us into a level of support we never intended.

@bsutton
Copy link

bsutton commented Sep 29, 2021 via email

@lrhn
Copy link
Member

lrhn commented Sep 29, 2021

The purpose of marking the API deprecates was precisely to leave things alone until we have a viable alternative, which is also what the documentation states. (Well, unless unexpected events change the world and our priorities in a way we haven't predicted, which is a caveat in everything.)

It's now documented that we are leaving the library alone (which includes having effectively no maintenance or support), and we discourage other users from starting to use the feature (basically asking them to leave it alone too).
There are no active plans to remove the feature. There are also currently no active process for finding a viable alternative, which means that it's unlikely that we do anything in the near future.

I'm very open to suggestions for more viable alternatives. Just be aware that allowing synchronous functions to take asynchronous breaks is the problem here, not just the implementation. No matter how convenient it might be, it's fundamentally breaking the assumptions we've told people they can make about synchronous computation.

(I'd expect blocking the isolate and computing the async computation in a separate isolate to be the most immediate alternative to waiting for an asynchronous computation in a synchronous function, but any other ideas are welcome too.)

copybara-service bot pushed a commit that referenced this issue Oct 27, 2021
Clarify this is supported, but deprecated, and may get removed
in the future should we find a suitable replacement.

Bug: dart-lang/api.dart.dev#81
Bug: #39390
Change-Id: I1304a0862986ae1b365a063ee75f0041341523f5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214620
Commit-Queue: Michael Thomsen <mit@google.com>
Reviewed-by: Lasse R.H. Nielsen <lrn@google.com>
@mit-mit
Copy link
Member

mit-mit commented Oct 28, 2021

Closing as resolved for now.
https://api.dart.dev/be/182485/dart-cli/dart-cli-library.html

We understand the concerns raised. While this isn't as clear as we -- and several ecosystem members -- would have liked, I believe it's an acceptable outcome for now: As Lasse said, we'll leave things as they are until a time where we have found a viable alternative.

@mit-mit mit-mit closed this as completed Oct 28, 2021
@vsmenon
Copy link
Member

vsmenon commented Apr 6, 2023

Describing waitFor() as "in contradiction with Dart's event model" is too strong—plenty of languages support an event-loop based model while also being compatible with synchronous calls. This is the basis of the concept of fibers, for example, and very similar to how Go's goroutines work. waitFor() is just an implementation of the special case of that concept with a single stack shared between all events.

For what it's worth, I strongly believe full fiber support on the VM would be a substantial benefit to Dart.

@nex3 how does the code that depends on waitFor work on node? I recall that you could run dart-sass in two modes: native and translated to JS. Is this still the case?

In JS mode, we currently use the fibers package. We're exploring the possibility of switching to worker threads along with the Atomics.wait() function, although whether we do so in practice depends on how high the overhead of spawning a thread turns out to be.

@nex3 - what's the current state of this? It appears that fibers no longer works in Node / v8.

@nex3
Copy link
Member

nex3 commented Apr 6, 2023

For the Node.js embedded host, we use Atomics and receiveMessageOnPort(). We run the subprocess in a separate worker thread and send its events through a MessagePort, while using a SharedArrayBuffer along with Atomics.wait() and Atomics.compareExchange() to signal to the main thread when there's an event ready to read from that port.

For Dart Sass compiled to JS, we give users three options: use the Node.js embedded host instead, use the synchronous API (which isn't compatible with most build system plugins), or accept a major performance hit. We've considered the possibility of pursuing a similar Atomics strategy, but converting all the API calls to something thread-serializable is non-trivial—we'd essentially be writing another copy of the embedded protocol.

@mraleph
Copy link
Member

mraleph commented Apr 21, 2023

Based on various internal discussions over the past month, I would like to move forward and remove waitFor in 3.2 (second stable release after 3.0) as we now have clear migration path for all users.

#52121 gives the details of the proposed timeline.

@mraleph mraleph mentioned this issue Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …). docs-api
Projects
None yet
Development

No branches or pull requests