dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
155 stars 43 forks source link

Random assertion failures `'_blockClosureRegistry.containsKey(id)': is not true` #1571

Closed stuartmorgan closed 1 month ago

stuartmorgan commented 1 month ago

In my video_player experiment, I not infrequently (especially when switching pages, so a player instance is being eliminated), get an assertion failure like this:

[ERROR:flutter/runtime/dart_vm_initializer.cc(40)] Unhandled Exception: 'package:objective_c/src/internal.dart': Failed assertion: line 345 pos 10: '_blockClosureRegistry.containsKey(id)': is not true.
#0      _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:50:61)
#1      _AssertionError._throwNew (dart:core-patch/errors_patch.dart:40:5)
#2      getBlockClosure (package:objective_c/src/internal.dart:345:10)
#3      _ObjCBlock_ffiVoid_closureTrampoline (package:video_player_avfoundation/src/ffi_bindings.dart:787:11)
#4      ObjCBlock_ffiVoid.listener.<anonymous closure> (package:video_player_avfoundation/src/ffi_bindings.dart)
#5      _RootZone.runUnaryGuarded (dart:async/zone.dart:1594:10)
#6      _RootZone.bindUnaryCallbackGuarded.<anonymous closure> (dart:async/zone.dart:1633:26)
#7      _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:184:12)

I may be doing something wrong, but I don't know what, or how to investigate/debug, since the exception doesn't give me any information. It seems like a lifetimer error, maybe? But I thought .listener would handle lifetime for me.

liamappelbe commented 1 month ago

It seems like a lifetimer error, maybe?

Yep, this assert means the callback is being deleted before it's invoked.

But I thought .listener would handle lifetime for me.

Yeah, it's supposed to. The Dart wrapper object holds a reference to the block that should keep it alive. If you're passing the block to an ObjC API that holds a reference to it, that should also keep the block alive. I'm assuming you're passing the block to an ObjC API then dropping the Dart wrapper object.

One possibility is that there's a bug in the ref counting logic. Eg the ObjC API might not increment the refcount, instead expecting to be passed a callback that already has the count incremented. If this bug is flaky, then this seems unlikely to be the issue.

Another possibility is that the Dart wrapper object is being GC'd before the ObjC API receives the block. As part of #1442 I had to change how the Finalizable interface is applied to the wrapper objects. Maybe that introduced a bug that allows the Dart GC to collect these objects early. Though if that were the case, I'd expect the ObjC API to crash when it tries to retain the reference.

A third possibility is that the ObjC API invokes the callback, then immediately release its reference. Both the invocation and the block destructor send a message to the owner isolate. If the destructor message is handled before the invocation message, then you'd see exactly this error. I didn't think messages could be handled out of order like that, but maybe Dart_PostInteger_DL that the dtor uses and the internal message posting method that the invocation uses have a different priority? The fix would be easy enough, just adjust the listener trampolines so they increment the ref count of the target block, as well as the block's arguments. I'll see if I can repro this case.

Might a duplicate of https://github.com/dart-lang/native/issues/1570

liamappelbe commented 1 month ago

Managed to repro third possibility, which is good because it's the easiest to fix 😆.

dupuchba commented 1 month ago

FYI I also had the problem (it's ClojureDart put it compiles exactly the same in this case)

(-> (ns_financekit_bindings/FinanceKitBridge.new1)
    (.requestAuthorizationWithCompletionHandler_
      (ns_financekit_bindings/ObjCBlock_ffiVoid_NSString.listener
        (fn [a]  "never called, throw before"))))

error ->

flutter: 'package:objective_c/src/internal.dart': Failed assertion: line 345 pos 10: '_blockClosureRegistry.containsKey(id)': is not true.
flutter: 
#0      _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:50:61)
#1      _AssertionError._throwNew (dart:core-patch/errors_patch.dart:40:5)
#2      getBlockClosure (package:objective_c/src/internal.dart:345:10)
#3      _ObjCBlock_ffiVoid_NSString_closureTrampoline (package:Paktol/ns_financekit_bindings.dart:125:11)
#4      ObjCBlock_ffiVoid_NSString.listener.<anonymous closure> (package:Paktol/ns_financekit_bindings.dart)
#5      _RootZone.runUnaryGuarded (dart:async/zone.dart:1594:10)
#6      _RootZone.bindUnaryCallbackGuarded.<anonymous closure> (dart:async/zone.dart:1633:26)
#7      _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:184:12)
dupuchba commented 1 month ago

update: it works If I follow your recommendation to use a global handler

stuartmorgan commented 1 month ago

It's anecdotal, but FWIW I haven't been able to repro since picking up that last commit in my experiment, and I used to be able to repro it pretty easily. Looks like the case you fixed was indeed what I was hitting!