Dynamic unawaited_futures?

[eernstg]

The lint unawaited_futures is intended to flag situations where a future is the result of a computation in a function body marked async, and it is then discarded rather than being awaited. That situation is likely to be a bug, because it allows computations associated with that future to occur much later (rather than during the execution of an await on the future immediately when it is received, e.g., as the return value of a function invocation), and the reordering of side effects may give rise to bugs that are very subtle and hard to fix.

However, recent discussions (and older ones, actually) have demonstrated that there are many situations where a future is thus ignored, and it may even be impossible to detect the buggy situations among all those situations using static analysis. Cf. #58656, #58650, #57769, #57653.

This obviously gives rise to the question "Could we introduce a dynamic check that detects discarded futures?" A dynamic check could be complete in the sense that it could catch every discarded future, if we can characterize such futures.

Consider the following example:

Future<void> f() async {
  await null; // Force asynchronous execution.
  print('f runs!');
}

void main() {
  f(); // Ignores a future.
  print('main runs!');
}

This program prints 'main runs!' followed by 'f runs!'. The idea would be that the latter causes a dynamic error a la "Someone discarded a future!", rather than an execution of await fut where fut is the future which is returned by the execution of f (and discarded in main).

We might need the ability to execute a method canBeIgnored() (an extension method, presumably) on a future (presumably requiring it to be a system provided future, not an instance of a user-written class), and this would allow the future to be awaited after main has returned. For any future where this method wasn't executed, the dynamic error would occur.

Can we do this? @lrhn, what do you think?

[eernstg]

/cc @pq, @natebosch, @parren-google, based on some recent comments on the related issues.

[lrhn]

Could we introduce a dynamic check that detects discarded futures?

That sound undecidable. Probably undesirable. And definitely expensive!

I'd be vary about building a run-time check into the language.
As described here, it introduces a runtime error in some cases that currently are not errors, which feels more like a trip-wire than a helpful feature. We have generally been moving towards features that turn runtime errors into static errors, so introducing new runtime errors feel like a step in the wrong direction. (Not completely, we also like "fail early", which this might be, but only if it doesn't have too many false positives.)

It's not clear which behavior is actually being proposed. It's not even clear what it means to ignore a future.

... it could catch every discarded future, if we can characterize such futures.

That's a very big "if". I have no idea where to even begin.

Would it only apply to async functions, like the current lint? Those will eventually return a future, so it's usually possible to draw some line from the incoming future to the outgoing future.
A non-async function won't return a future, so if it contains a future anywhere, that future is somehow dropped.
Probably only after doing "something" to it.
Is calling .then on a future "using" it? Is calling .toString on a future using it?
Is assigning a future to a variable using it? What if the variable isn't read again? What if the variable has type void?
Is adding a future to a collection using it? What if the collection gets GC'ed?

(All futures are eventually ignored, usually after their callbacks have triggered. The code that runs those callbacks has a reference to the future, and it does noting with that future after reading out the state and result.)

Unless we make GC check whether a future "has been used", we can't definitely say anything.
(But if a future gets GC'ed before you attach an error handler to it, then it definitely won't complete with an error later, so that's probably safe. It's only when someone keeps a hold on the other end of the future that you shouldn't drop it.)

Without involving GC, we'd have to insert checks where a future might be dropped.
Since this isn't static, I'll assume we can't rely on types. We'd have to check any value which can be a future, at any place where that value is "lost" (assigned to void, dropped on the floor after not being assigned to a variable, and ... other places).
And since Future isn't sealed, that'll be everywhere any value isn't "used".

Sometimes, a future should be ignored. For example, the future returned by void foo() async { ... } is intended to be ignored.
Other functions return futures that you may or may not want to wait for, depending on whether you need to know when the operation ends or not. If you don't care, it's fine (and documented as so) to not wait for the future.

It's also very hard to ensure that a future is not dropped without completely disallowing assignment from Future to dynamic/Object/void. After assigning to Object, we have no clues left about something being a future or not.
So, no up-casts allowed.
(Which means that there is a very simple solution, which ensures that futures are not dropped: Make Future not be a subtype of Object, and not allow them in places where a value isn't used at all. Then there's nowhere to drop them!)

Without that, the current void unawaited(Future<Object?> _) {} works perfectly well to ignore a future.

(But, if someone can actually specify "unused future" in a way that makes sense, I'd be happy to see that!)

[parren-google]

Given the challenges - and assuming something like "make Future not be a subtype of Object" is not an option - I would be pragmatic here and focus on static analysis flagging the pitfalls that have been shown to strike again and again, especially where the language makes the pitfall very hard to spot to both the author and the code reviewer. The key problem to me is that calling a method returning a new future looks exactly like one that doesn't. So I would focus on usages of methods returning futures (because usually these are new futures). I would not focus on futures put into variables, taken from structures, etc.

So these shoud be covered (assuming Future someAsync() async {} and void takesVoidFn(void fn())):

• A returned future is dropped straightaway, as in foo() async { someAsync(); }.
• A returned future will clearly be dropped if the method is called, as in takesVoidFn(someAsync);.

But not:

• var x = someasync(); because that also gives me an unused variable lint. Or, if I do something with it, it is likely to lead to a typing error. And I already indicated I understand something important is coming back.

If there are frequent pitfalls related to variables holding futures, we could also consider a linter that disallows inference of the Future type for variables, but add syntax like Future<?> x = someAsync(); to still allow inference of the return type of the Future.

If we are open to more radical changes, then we could consider a new type, like HeldFuture which does extend Object and can live in containers, and the regular Future would no longer extend Object, and have the connotation of "future that needs to be acted on", either by awaiting it, or by explicitly converting it into a HeldFuture (for example via a getter .started). We could then even consider forbidding variables to have the type Future, so that I would have to write var x = someAsync().started;.

[eernstg]

@lrhn, you're assuming a lot of magic. There is no proposal here that requires solving the halting problem. So let me explain a little more about how simple this proposal actually is. ;-)

Note that this is exclusively intended to be used during debugging and test runs, it is not likely to be useful in deployed systems. The intention is that it should reveal that some future related computations are delayed because they are never awaited. The assumption is that a noteworthy subset of the Dart applications out there will be designed to use asynchrony, but not to have any code executed after main returns. In other words "every future should be awaited at some point". This mechanism would detect violations of that property.

We consider each program execution to consist of two phases: (1) Execution of main, and (2) execution that takes place after the execution of main. During phase (2), execution of code that will give rise to completion of a Future that isn't marked canBeIgnored will throw rather than being executed.

In the example, print('f runs!') occurs during phase 2, and it is part of a computation that will give rise to completion of a future, so it would throw rather than run.

(Actually, it might be even simpler if we tie the run-time error directly to the completion of a future: If we're after main, and we're about to complete a future, and it isn't marked canBeIgnored, then we throw. That's it.)

It's a little bit like a memory leak checker: For C/C++ applications whose design is intended to maintain the invariant that every piece of dynamically allocated memory must be freed exactly once during the time where main runs, it may be possible to detect that some pieces of memory have not been freed when main returns. It could be difficult to track down exactly why that piece of memory was leaked, but knowing that there was a leak at all is an important first step, and it might be possible to use properties of that memory block to start tracking down how it happened.

There are lots of issues that don't exist:

it introduces a runtime error in some cases that currently are not errors

During debugging, getting a run-time error which serves as a witness that something went wrong is better than not knowing that there is a problem at all.

Would it only apply to async functions

We may wish to subset the mechanism, but as a starting point it would cause execution of code that leads to completion of any future to throw if it occurs after main has returned, and the future isn't marked canBeIgnored. If it is helpful to single out the futures returned by execution of an async body then we should be able to, e.g., mark all futures created by user-accessible constructors as canBeIgnored (which may or may not be a useful thing to do, but we can).

A non-async function won't return a future, so .. [it was] somehow dropped.

Right, that was the example I gave, using a non-async main (well, a non-async function can of course return a future, but it couldn't await it). The execution of print('f runs!') took place after main returned, as expected, and that's the time where I proposing we could raise an error. No halting problems here, and no GC!

The code that runs those callbacks has a reference to the future

That's basically the reason why I think we can raise an error.

Unless we make GC check whether a future "has been used"

I don't see why we would need to consider GC: When print('f runs!') runs, it's because that piece of code is associated with an object which is reachable, and the future is reachable from there, so it hasn't been garbage collected.

Since this isn't static, I'll assume we can't rely on types

If it were a problem to keep track of the futures whose associated computation will be executed after main returns, why is it not a problem to run those computations after main returns? In other words, it isn't a problem, because we do have a reference to every future which is associated with a computation that will occur after main returns.

Sometimes, a future should be ignored

For that we have thatFuture.canBeIgnored(). We would presumably make it part of the generated code for a void async function that it immediately calls canBeIgnored on the future that it will return. The computations needed to complete those futures would then happily proceed after main returns (e.g., writing to log files could be unimportant for the program logic, so it's not a bug to do it after main).

no up-casts allowed ..
Make Future not be a subtype of Object

If this were a problem then it would also prevent running the code after main returns! There is absolutely no need to pull Future out of the Object? hierarchy.

[eernstg]

@parren-google wrote:

I would be pragmatic here and focus on static analysis

I certainly think we should use static analysis, too: It flags locations where the code is likely (or guaranteed) to discard an object which is known to be a future, so we get an early heads up, near the source of the problem.

The dynamic approach which is the topic of this issue complements the static analysis: It detects likely bugs at a very late point in time (which is inconvenient), but it is complete in the sense that it will report every case where a non-canBeIgnored future is about to be completed after main.

So it's again a little bit like the memory leak detection: The dynamic approach will detect that there is a problem no matter how it happened, whereas a static analysis is guaranteed to give up before it solves the halting problem. Knowing that there is a problem at all, we'd need to do all kinds of smart debuggy things to detect how it happened. The ability to create cross-asynchrony stack traces could be very helpful here.

[lrhn]

@eernstg That makes some sense. After main returns (and assuming main is async, that means after the future returned by main completes), the program should be done. Anything asynchronous that happens after that is considered an "async leak".
We could probably do that today by introducing a zone s.t. any zone.run or execution of any zone.scheduleMicrotask or zone.createTimer callbacks, which happens after main has completed, will just refuse to do anything other than throwing.

I don't actually think that approach will work well for real programs. The main method is not the base of all computation, it's a perfectly reasonable thing to do to just initialize things in main, and rely on events to drive the rest of the program. That's the fundamental operating mode of reactive programs (UI-based apps for example). The main method is long gone when the real program starts running.
The same issue happens if main is not async, and it spins up an asynchronous computation before it exits normally.
Everything else happens after main has returned. The return of main is not really a meaningful point in an asynchronous program's execution, not unless the program is deliberately written to make it so.

So yes, I assumed magic, because this was not an option I even considered. Anything that would actually be correct, would be magic.

[eernstg]

It is true that some programs will initiate some asynchronous computations and rely on post-main execution. However, developers of those programs would then simply never enable this feature.

So the question is: How large is the set of programs for which all post-main computation is actually considered to be a bug (an async leak)? If that's a reasonably large set of programs then it may be worthwhile.

But it also sounds like we could just make it a programming idiom: If you want to detect async leaks then you need to have these three lines of code in your main (whatever it takes). It would then simply be a piece of good advice that could be mentioned somewhere in the Dart documentation, and anyone who wants to use this (during debugging, presumably) would add those three lines of code.

[lrhn]

It should be fairly simple to make such a Zone which prevents callbacks after some flag has been set.
Something like:

import "dart:async";

Future<T> preventAsyncLeak<T>(Future<T> Function() action) {
  bool done = false;
  void checkNotDone() {
    if (done) throw StateError("Asynchronous computation after end");
  }
  return runZoned(() => action().whenComplete(() {
    done = true;
  }), zoneSpecification: ZoneSpecification(
    registerCallback: <R>(self, parent, zone, f) {
      return parent.registerCallback(zone, () {
        checkNotDone();
        return f();
      });
    },
    registerUnaryCallback: <R, S>(self, parent, zone, f) {
      return parent.registerUnaryCallback(zone, (a) {
        checkNotDone();
        return f(a);
      });
    },
    registerBinaryCallback: <R, S, T>(self, parent, zone, f) {
      return parent.registerBinaryCallback(zone, (a, b) {
        checkNotDone();
        return f(a, b);
      });
    }
    run: <R>(self, parent, zone, f) {
      checkNotDone();
      return parent.run(zone, f);
    },
    runUnary: <R, T>(self, parent, zone, f, a) {
      checkNotDone();
      return parent.runUnary(zone, f, a);
    },
    runBinary: <R, S, T>(self, parent, zone, f, a, b) {
      checkNotDone();
      return parent.runBinary(zone, f, a, b);
    },
  ));
}

/// Example
void main() {
  preventAsyncLeak(() async {
    scheduleMicrotask(() {
      print("Microtask!");  // Is printed
      scheduleMicrotask(() {
        print("Microtask 2!");  // Throws before printing.
      });
    });
  });
}

It'll probably not catch everything (Platform async code won't use registerCallback on every callback, so I added the run functions too), but I think it should catch a lot.

[eernstg]

I think it should catch a lot.

Cool, thank you!

[eernstg]

So here's a concrete approach which will do what I had in mind:

The basic assumption is that some asynchronous programs are designed to await all their futures during the execution of main. If a future is discarded without being awaited then it is a bug (by definition, because that's how the program was designed), and we want to make it a run-time error, such that we know that there was an "async leak".

So let's say that we have a program where an async leak is defined to be a bug. Here's a simple example:

// Stored in 'main.dart'.

Future<void> f() async {
  await null; // Force asynchronous execution.
  print('f runs!');
}

void g() => f(); // Ignores a future.

Future<void> main() async {
  g();
  print('main runs!');  
}

We can run this program (dart main.dart), and it prints 'main runs!' followed by 'f runs!'. So the execution succeeds (there are no run-time errors), but we know that it is a bug that 'f runs!' is executed after main returns. We should have awaited f() in g, so g should also have been async.

With real-world software, of course, we may not be able to detect that this kind of bug exists, because the post-main computation may not have any visible effects, and/or we may not be able to see exactly when main terminates.

So we apply the following standard debugging technique. We write a tiny wrapper program:

// Stored in 'debugMain.dart'.

import 'prevent_async_leak.dart';
import 'main.dart' as mainLib;

void main() => preventAsyncLeak(mainLib.main);

The library 'prevent_async_leak.dart' contains the following, basically taken from @lrhn's example here:

prevent_async_leak.dart

import "dart:async";

Future<void> preventAsyncLeak<T>(Future<void> Function() main) {
  bool done = false;
  void checkNotDone() {
    if (done) throw StateError("Asynchronous computation after end");
  }

  var action = () async => scheduleMicrotask(main);

  return runZoned(
      () => action().whenComplete(() {
            done = true;
          }),
      zoneSpecification: ZoneSpecification(
        registerCallback: <R>(self, parent, zone, f) {
          return parent.registerCallback(zone, () {
            checkNotDone();
            return f();
          });
        },
        registerUnaryCallback: <R, S>(self, parent, zone, f) {
          return parent.registerUnaryCallback(zone, (a) {
            checkNotDone();
            return f(a);
          });
        },
        registerBinaryCallback: <R, S, T>(self, parent, zone, f) {
          return parent.registerBinaryCallback(zone, (a, b) {
            checkNotDone();
            return f(a, b);
          });
        },
        run: <R>(self, parent, zone, f) {
          checkNotDone();
          return parent.run(zone, f);
        },
        runUnary: <R, T>(self, parent, zone, f, a) {
          checkNotDone();
          return parent.runUnary(zone, f, a);
        },
        runBinary: <R, S, T>(self, parent, zone, f, a, b) {
          checkNotDone();
          return parent.runBinary(zone, f, a, b);
        },
      ));
}

This program runs for a while, prints `main runs!', and then throws (just when it's about to print 'f runs!'). So we have now detected that there is an async leak in this program.

Fixing the bug is a matter of normal debugging, so we don't get any particular help for that. But the point is that it is useful to know that this bug exists at all.