Crash "runtime/vm/object.cc: 20076: error: expected: !IsFinalized()" on pkg/vm/test/incremental_compiler_test

[alexmarkov]

The test was flaky but turned into RuntimeError on pkg-linux-debug bot recently. From the log:

vm stderr: ../../runtime/vm/object.cc: 20076: error: expected: !IsFinalized()

vm stderr: version=2.13.0-edge.cf7587a160df9a57d5294a8bccbdfc60480d2190 (be) (Tue Apr 6 11:49:09 2021 +0000) on "linux_x64"

vm stderr: pid=18914, thread=18919, isolate_group=main(0x564b67673000), isolate=(nil)((nil))

vm stderr: isolate_instructions=564b64e5ce40, vm_instructions=564b64e5ce40

vm stderr:   pc 0x0000564b6524fddc fp 0x00007f84d4bebe30 dart::Profiler::DumpStackTrace(void*)+0x7c

vm stderr:   pc 0x0000564b64e5cff4 fp 0x00007f84d4bebf10 dart::Assert::Fail(char const*, ...)+0x84

vm stderr:   pc 0x0000564b651f0eac fp 0x00007f84d4bebf70 dart::Type::IsInstantiated(dart::Genericity, long, dart::ZoneGrowableHandlePtrArray<dart::AbstractType const>*) const+0x25c

vm stderr:   pc 0x0000564b651a31fd fp 0x00007f84d4bebfd0 dart::FunctionType::IsInstantiated(dart::Genericity, long, dart::ZoneGrowableHandlePtrArray<dart::AbstractType const>*) const+0xed

vm stderr:   pc 0x0000564b651f079e fp 0x00007f84d4bec000 dart::FunctionType::SetIsFinalized() const+0x2e

vm stderr:   pc 0x0000564b650583fc fp 0x00007f84d4bec070 dart::ClassFinalizer::FinalizeSignature(dart::Zone*, dart::FunctionType const&, dart::ClassFinalizer::FinalizationKind, dart::ZoneGrowableHandlePtrArray<dart::AbstractType const>*)+0x2bc

vm stderr:   pc 0x0000564b650588c3 fp 0x00007f84d4bec0e0 dart::ClassFinalizer::FinalizeMemberTypes(dart::Class const&)+0x3d3

vm stderr:   pc 0x0000564b65058fdd fp 0x00007f84d4bec190 dart::ClassFinalizer::FinalizeClass(dart::Class const&)+0x1ad

vm stderr:   pc 0x0000564b6505ab83 fp 0x00007f84d4bec2c0 dart::ClassFinalizer::LoadClassMembers(dart::Class const&)+0x143

vm stderr:   pc 0x0000564b65179e50 fp 0x00007f84d4bec350 dart::Class::EnsureIsFinalized(dart::Thread*) const+0xf0

vm stderr:   pc 0x0000564b65235402 fp 0x00007f84d4bec3e0 dart::Class::CheckReload(dart::Class const&, dart::ProgramReloadContext*) const+0xc2

vm stderr:   pc 0x0000564b6511ff68 fp 0x00007f84d4bec490 dart::ProgramReloadContext::ValidateReload()+0x328

vm stderr:   pc 0x0000564b65119e8a fp 0x00007f84d4bec720 dart::IsolateGroupReloadContext::Reload(bool, char const*, char const*, unsigned char const*, long)+0xaca

vm stderr:   pc 0x0000564b6510faa8 fp 0x00007f84d4bec7b0 dart::IsolateGroup::ReloadSources(dart::JSONStream*, bool, char const*, char const*, bool)+0x1f8

vm stderr:   pc 0x0000564b652dc18f fp 0x00007f84d4bec7f0 /b/s/w/ir/cache/builder/sdk/out/DebugX64/dart-sdk/bin/dart+0x207918f

vm stderr:   pc 0x0000564b652d2d35 fp 0x00007f84d4bec960 dart::Service::InvokeMethod(dart::Isolate*, dart::Array const&, bool)+0x4a5

vm stderr:   pc 0x0000564b652d38be fp 0x00007f84d4bec990 dart::Service::HandleIsolateMessage(dart::Isolate*, dart::Array const&)+0x1e

vm stderr:   pc 0x0000564b6510c211 fp 0x00007f84d4becb80 dart::IsolateMessageHandler::HandleMessage(std::__2::unique_ptr<dart::Message, std::__2::default_delete<dart::Message> >)+0x371

vm stderr:   pc 0x0000564b65150c91 fp 0x00007f84d4becc10 dart::MessageHandler::HandleMessages(dart::MonitorLocker*, bool, bool)+0x221

vm stderr:   pc 0x0000564b65151bbe fp 0x00007f84d4becc90 dart::MessageHandler::TaskCallback()+0x35e

vm stderr:   pc 0x0000564b6531153b fp 0x00007f84d4becd30 dart::ThreadPool::WorkerLoop(dart::ThreadPool::Worker*)+0x1cb

vm stderr:   pc 0x0000564b65311f79 fp 0x00007f84d4becd80 dart::ThreadPool::Worker::Main(unsigned long)+0x179

vm stderr:   pc 0x0000564b65248049 fp 0x00007f84d4beced0 /b/s/w/ir/cache/builder/sdk/out/DebugX64/dart-sdk/bin/dart+0x1fe5049

--- Re-run this test:
python tools/test.py -n unittest-asserts-debug-linux pkg/vm/test/incremental_compiler_test

Full log: https://dart-ci.appspot.com/log/pkg-linux-debug/unittest-asserts-debug-linux/10261/pkg/vm/test/incremental_compiler_test

@aam @crelier

[crelier]

The assert fault seems to indicate that a class was augmented with additional type parameters, which should not be an edit supported by hot reload. Could that be the problem?

[aam]

@crelier , the test https://github.com/dart-lang/sdk/blob/master/pkg/vm/test/incremental_compiler_test.dart#L956 does add another type argument that should result in rejected hot reload.
The check for change in type arguments count happens https://github.com/dart-lang/sdk/blob/master/runtime/vm/object_reload.cc#L733 after new class is attempted to get finalized https://github.com/dart-lang/sdk/blob/master/runtime/vm/object_reload.cc#L662. It's during that finalization we fail an assertion.

[aam]

We could something along the lines of https://dart-review.googlesource.com/c/sdk/+/195305 as a fix.

[crelier]

@aam The check that you point at ends up executing this line https://github.com/dart-lang/sdk/blob/master/runtime/vm/object_reload.cc#L787, which does not look right to me. Calculating the number of type arguments requires finalization. We should check the number of type parameters instead, which is much simpler:
if (NumTypeParameters() != replacement.NumTypeParameters()) {

Would you mind trying that?

[aam]

When we do the check assumption is that class is finalized already, right?

[crelier]

@aam Because of the possibility of type argument overlap between the super class and derived class, the number of type arguments could be incompatible, although the number of type parameters has not changed. Maybe that was the reason for checking the type arguments. I think we should check both, making sure to check the type parameters first:

if (NumTypeParameters() != replacement.NumTypeParameters() ||
    NumTypeArguments() != replacement.NumTypeArguments()) {

When we do the check assumption is that class is finalized already, right?
I am not familiar with hot reload, so I am no sure.

[aam]

When we do the check assumption is that class is finalized already, right?

I am not familiar with hot reload, so I am no sure.

We seem to check type arguments only when class if finalized: this is how the function is called https://github.com/dart-lang/sdk/blob/master/runtime/vm/object_reload.cc#L733