Bug with late Finalizable objects

[liamappelbe]

In some cases, a late Finalizable will incorrectly report that it's uninitialized:

import 'dart:ffi';

import 'package:test/test.dart';

class Foo implements Finalizable {
  final int x;
  Foo(this.x);
}

void main() {
  late Foo foo;

  setUpAll(() {
    foo = Foo(123);
  });

  test('check foo', () {
    expect(foo.x, 123);
  });
}

Unhandled exception:
LateInitializationError: Local 'foo' has not been initialized.
#0      LateError._throwLocalNotInitialized (dart:_internal-patch/internal_patch.dart:199:5)
#1      main (late_finalizable.dart)
#2      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:297:19)
#3      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:192:12)

This test passes if you remove implements Finalizable, or change the variable to Foo? foo;.

cc @dcharkes

[dcharkes]

This is an interesting case, in general late variables can be captured before they are initialized.
The Finalizable transformation wants to keep all variables alive in scope.
However, here it tries to keep a variable alive that has not been initialized yet.
And that causes a load from the late field, which causes the check whether it is initialized to be added.

Smaller repro:

class Foo implements Finalizable {}

void main() {
  late Foo foo;
  foo = Foo();
  print(foo);
}

The kernel transformation inserts reachabilityFences:

static method main() → void {
  late self::Foo foo;
  foo = block {
    final self::Foo :expressionValueWrappedFinalizable = new self::Foo::•();
    _in::reachabilityFence(foo); // This fence is executed before the variable is intialized.
  } =>:expressionValueWrappedFinalizable;
  // ...
}

In the VM (in unoptimized), the block with the reachabilityFence gets compiled as

    t0 <- LoadLocal(foo @-1)
    StaticCall:34( reachabilityFence<0> t0, recognized_kind = ReachabilityFence)

But the LoadLocal gets a check whether it is initialized in StreamingFlowGraphBuilder::BuildVariableGetImpl:

B3[target]:22
    t0 <- Constant(#foo)
    StaticCall:28( _throwLocalNotInitialized@9040228<0> t0)
    goto:30 B5
B4[target]:24
    goto:32 B5
B5[join]:26 pred(B3, B4)
    t0 <- LoadLocal(foo @-1)
    StaticCall:34( reachabilityFence<0> t0, recognized_kind = ReachabilityFence)

Option 1: Basically, we don't want to throw the exception if we load the local and pass it to a reachability fence.
However, recognizing this pattern in IL might be costly. (edit maybe not: if we can recognize the static call to reachability fence when reading from kernel we know the expression will always be a variable get, because that is what we emit in the transform.)

Option 2: Alternatively, we could consider trying to not insert the fence. But that would break the semantics of Finalizable:

late Foo foo;
foo = Foo();
foo = longMethodThatUsesTheNativeResourceFromTheFirstFooAndReturnsANewFoo();

Option 3: disallow late Finalizables. That seems overly restrictive for a marker interface that is going to be tagged on various classes.

I'll take a stab at option 1 and see how costly it would be to recognize these patterns.

cc @lrhn from a language POV. What is the semantics of a Finalizable with late? "If the variable has been initialized, the value is keept alive until the end of the scope"?

cc @mraleph thoughts? ideas? (Besides that your alternative implementation idea would probably not have had this issue. 😅 )

[dcharkes]

Fix underway: https://dart-review.googlesource.com/c/sdk/+/244628

Thanks for reporting @liamappelbe!