Lint to warn when possibly long lived closures capture unexpected variables on the VM #58365
Description
Describe the rule you'd like to see implemented
Having closures capture Dart objects is a common source of memory leaks in Flutter applications. Harmless looking code can lead to memory leaks because due to a long lived closure that capture variables that Dart users do not expect them to capture to to implementation details of the DartVM.
Dart2JS does not suffer from the same memory leaks and it is possible that the DartVM will be optimized in the future to avoid this sort of leak at which point this lint can be removed. It would be ideal if the VM could instead be changed to avoid this issue but that may be expensive to implement or have undesirable performance issues in the general case.
Common cases:
PageRouteBuilder widgets are a good first case to lint for as the closure defining the builder is virtually guaranteed to live on past when the widget that created the route builder should.
Related case: BuildContext objects should not be kept alive past an async gap (see #58215) or otherwise used after the buildContext might have been unmounted.
In the general case it is challenging to distinguish between variables captured by a closure that will not lead to memory leaks and ones that will.
Examples
class MemoryTest extends StatelessWidget {
final MyBigObject bigObj = MyBigObject();
MemoryTest(this.bar);
final bar;
@override
Widget build(BuildContext context) {
var foo = 42;
return OutlinedButton(
child: Text("PageRouteBuilder"),
onPressed: () {
Navigator.of(context).pushReplacement(PageRouteBuilder(
pageBuilder: (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) => MemoryTest(foo),
));
onCaptureThis: () {
print("This=$this, foo=$foo");
},
);
}
}Desired lint error:
The pageBuilder closure captures context and this due to () and is passed as a parameter to a PageRouteBuilder which is likely to be long lived. Fix the issue by stopping the closure from capturing these variables that are not referenced in the closure.
Code after quick fix:
class MemoryTest extends StatelessWidget {
final MyBigObject bigObj = MyBigObject();
MemoryTest();
@override
Widget build(BuildContext context) {
var foo = 42;
return OutlinedButton(
child: Text("PageRouteBuilder"),
onPressed: () {
Navigator.of(context).pushReplacement(PageRouteBuilder(
pageBuilder: _createPageBuilder(foo);
onCaptureThis: () {
print("This=$this, foo=$foo");
},
);
}
static Widget _createPageBuilder(int foo) {
return (BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) => MemoryTest(foo)
}
}Details
In this example, the pageBuilder closure actually captures both context and this (MemoryTest). That is surprising to the point that advanced users are unlikely to be able to root cause the issue. The problem is that the VM implementation will capture those variables as part of the pageBuild closure because of the unrelated onCaptureThis closure that happens to capture this and that the onPressed closure captures context.
The onCaptureThis closure wouldn't generate the lint as we don't have evidence that the callback from the onCaptureThis closure is long lived.
Additional context
flutter/flutter#79605 (comment)
Is an example of a Flutter bug that stumped a number of experienced Flutter developers who weren't expecting Flutter's behavior around capturing this. A lint that specifically warns that Closures passed to classes where the Closure could be long lived could have helped clarify the problem.