linter icon indicating copy to clipboard operation
linter copied to clipboard

Possible memory leak in use_late_for_private_fields_and_variables

Open bwilkerson opened this issue 3 years ago • 5 comments

The visitor for use_late_for_private_fields_and_variables defines two static fields. They are used to keep track of information across visitors when a library has multiple compilation units (that is, has parts).

While it makes sense to allow a lint to examine all of the compilation units in a library, the current approach is susceptible to a memory leak because entries are only removed from the map when all of the units in the library have been visited. But (a) something could happen to prevent some of the units from being linted, or (b) an exception could occur when analyzing the last unit in the library that prevents the clean-up code from being reached.

I believe that the right way to support this kind of lint is to make these fields be instance fields and to visit all of the compilation units in the library at the same time. However, I'm not sure that the linting framework is written to allow diagnostics to be reported against anything other than the current compilation unit, so that might not be practical, and we might need a different approach.

And, assuming a safe implementation strategy is found, this should be done in a way that makes it easier to write other cross-library lints in the future.

bwilkerson avatar Jul 29 '22 17:07 bwilkerson

There are no tests for the part support, so those will need to be added when a fix is made.

srawlins avatar Aug 11 '22 15:08 srawlins

This support might already be broken. When I analyze these two files in an integration test:

// library.dart:

part 'part.dart';

int? _i2; // LINT

// part.dart:

part of 'library.dart';

m2() {
  _i2 = 1;
}

I get the following output:

/Users/srawlins/code/dart-linter/test_data/integration/use_late_for_private_fields_and_variables/part.dart 6:1 [lint] Use late for private members with a non-nullable type.

^

2 files analyzed, 1 issue found, in 3819 ms.

srawlins avatar Aug 11 '22 16:08 srawlins

Is this still a P1? Just asking, as it has been a while since the last update.

mosuem avatar Feb 14 '24 12:02 mosuem

Oops, we have completely dropped the ball on triaging whether P1 issues in this repo are being worked on. 😦

srawlins avatar Feb 14 '24 15:02 srawlins

No worries, I was just checking the status page on https://dart-pr-dashboard.web.app/ and saw this.

mosuem avatar Feb 14 '24 15:02 mosuem