Closed timsneath closed 3 years ago
This function should be removed. It is fundamentally incompatible with Dart's event loop model.
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...
If I recall correctly this was being used by @nex3 in SASS.
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.
Is the alternative to use then()
?
@rmacnak-google, how do we resolve this?
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?
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 rundart-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.
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 :)
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.
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.
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.
For my library dcli that would not work.
Dcli aims to replace bash using dart without burdening the developer with futures.
Making waitfor require an isolated would make dcli completely non viable as just about every function that dcli exposes use waitfor. I suspect that the proposed changes would make performance horrific.
Example code
if (!exists(somefile))
{
createDir(somedir);
touch (somefile);
'grep somedata anotherfile'.run;
}
Each of the above functions use waitfor.
Requiring the user to add await for each call is laborious and from experience dangerous. The danger is that if you forget an await you can end up causing havoc on your file system.
The original dcli library required the use of awaits and our internal team spent half their time searching for missing awaits. waitfor makes dcli a highly productive tool. Without it bash is probably a better solution.
From a user perspective I don't see the issue with waitfor. I understand it has its limitations but they are not affecting the code I'm using (except in a good way) so why are we trying to fix it.
On Wed, 13 Jan 2021, 7:22 am Nate Bosch, notifications@github.com wrote:
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.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dart-lang/sdk/issues/39390#issuecomment-758919007, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG32OAOUTUA7GTE3LTFQL3SZSVOXANCNFSM4JNUCWOQ .
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?
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.
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).
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 await
s sprinkled everywhere, and it doesn't seem to cause that much pain there.
In the end no one is forced to use waitfor.
Simply publish the risks and let those of us who need it, take those risks if we so choose.
We have over 100k lines of code in production that aggressively uses waitfor (via the dcli library) and the fact is that we haven't had a problem.
If there is a path to reduce the risk then great but don't go and break our code by removing a feature because it might break our code.
It reminds me of a discussion with a net admin I had year ago.
A denial of service bug was reported against some software we were using, so he bought the service down so that a possible dos attack couldn't bring the service down.
Face palm.
On Sat, 20 Feb 2021, 7:50 am Natalie Weizenbaum, notifications@github.com wrote:
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 https://www.npmjs.com/package/fibers 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.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dart-lang/sdk/issues/39390#issuecomment-782338831, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG32OHMPXK3Y4N2IQFPC3DS73FHXANCNFSM4JNUCWOQ .
If
callSomeMethod()
ends up runningwaitFor
, then we'll run a full event loop for a while, which might trigger events onsub
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?
@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.
@leafpetersen
Am I correct in thinking that "trigger events on
sub
" can only happen if some code running in the nested event loop accessessub
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).
@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 Fiber
s :smiley:.
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?
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.
I do want to see it supported.
If it's not supported then at some point it stops working.
Happy for it to be labelled with warnings but it's just too useful to not support.
On Tue, 23 Feb 2021 at 10:39, James D. Lin notifications@github.com wrote:
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.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dart-lang/sdk/issues/39390#issuecomment-783757408, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG32OGZXLHGQSMVM2HOOMLTALTLRANCNFSM4JNUCWOQ .
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.
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 insomeMethod
is running the full event loop, those events all get delivered, and thenull
handler initially attached tosub
throws the data away. Maybe there'll be adone
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?
@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.)
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.
Gotcha. I agree that the current API (wait on an existing future) isn't compatible with this model - what I was thinking was that perhaps that API is the wrong way to expose this functionality. That is, suppose what we exposed instead was "run this async callback on a fresh event and microtask queue with all existing async code paused". Would that solve the actual use cases? It allows you to take an async API and call it synchronously. As you observe, it does not allow you to take some pre-existing Future and wait on it.
Waiting on a pre-existing future is what waitFor
does, so it would be a different API.
Maybe it would be T waitForCall<T>(Future<T> operation())
which stops all queues, creates new ones, and then runs operation()
, expecting it to be all new asynchronous operations, and completing when the returned future completes.
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.
Asynchronous code does not have a strict tree-ordering of execution where you can cleanly separate events started in one piece of code from started in another piece of code. They all interact through synchronous code and global state which knows nothing about event loops, and by passing around futures, streams, ports and timers.
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)? The alternative is to put it into the new queue, which risks delivering port events out-of-order if another event was already in the paused queue (a definite no-go).
Same for timer events. We can pause all timers while the old event queue is paused (that would be hugely surprising to code which expects to be run), or we can let new timer events go into the new queue, but that risks events out of order..
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?
All in all, the design of waitFor
here is one of the better ways to make async code look synchronous without actually blocking the entire isolate. It's not something Dart was built for, and it can be misused (and I worry it won't scale), but I don't think doing less than continuing the event queue will be better. The alternative I'd prefer is fully blocking and waiting for a port event from another isolate. It costs (nothing else move while you wait), but you are also certain that the other isolate won't interfere with the current isolate otherwise. Then the seemingly synchronous code really would be synchronous and just using a blocking I/O operation.
@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.
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
.
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.
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.
@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.
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.
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
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.
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.
The problem with depreciation is that in two year time we are going to have the conversation; 'waitfor' has been deprecated for two years we can delete it now.
Depreciation should only occur if we have a replacement.
On Wed, 29 Sep 2021, 8:26 am Nate Bosch, @.***> wrote:
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.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dart-lang/sdk/issues/39390#issuecomment-929670073, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG32ODI2UF5EDLNXO2BVZTUEI6JVANCNFSM4JNUCWOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
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.
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.
On what metric are you assuming this is only used by a small no of users?
The vast majority of console apps are built for internal corporate usage and I'm not aware that dart collects any API usage statistics.
Whilst dcli's user base is small it is also growing.
The typical user of waitfor is also unlikely to be even aware 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.
I don't think you are listening to the right people.
On Wed, 29 Sep 2021, 9:02 am Vyacheslav Egorov, @.***> wrote:
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.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dart-lang/sdk/issues/39390#issuecomment-929685232, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG32OA2VM4TEKY5QSC6VKLUEJCQLANCNFSM4JNUCWOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
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.
This informs our decision of marking it deprecated. I don't see how that argument holds. My experience is that most users use an api with very little interaction with active issues unless it is causing them a problem. With Dcli, users won't even necessarily be aware that they are using waitfor as its use is buried in the dcli library. This is likely to be true of any other console apps that are published.
We've decided we don't want to make those promises because it carries a lot of risk.
It's this analysis that appears to be flawed and goes back to my statement about who you should be listening to.
The users of the api are saying it works just fine. It's only the theoreticians that are saying it carries risk and I haven't seen any code that demonstrates this in a real world scenario. As to being on the hook the statements appear to indicate that there is minimal maintenance burden on the code. Going forward as things stand doesn't appear to greatly increase the risk.
I would simply ask that you leave things alone until we have a viable alternative.
S. Brett Sutton Noojee Contact Solutions 03 8320 8100
On Wed, 29 Sept 2021 at 09:47, Nate Bosch @.***> wrote:
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 supporting this API if we make it officially supported. We've decided we don't want to make those promises because it carries a lot of risk.
The framing for the deprecation is not "The dart team changed their mind and decided not 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 to make clear 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.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dart-lang/sdk/issues/39390#issuecomment-929703538, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG32OD5OTSJKGBN3KMGPU3UEJHXLANCNFSM4JNUCWOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
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.)
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.
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 rundart-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 theAtomics.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.
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.
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.
https://github.com/dart-lang/sdk/issues/52121 gives the details of the proposed timeline.
The
waitFor
function has been marked as "experimental" for two years now: https://github.com/dart-lang/sdk/blob/ec8dcba611678ddff4bc11a7d521ad944323b444/sdk/lib/cli/wait_for.dart#L71(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!