dart-lang / native

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

[objective_c] Should we expose autorelease pools #1472

Open liamappelbe opened 1 month ago

liamappelbe commented 1 month ago

I'm using this pattern a lot in ffigen tests:

final pool = lib.objc_autoreleasePoolPush();
// Do stuff ...
lib.objc_autoreleasePoolPop(pool);

Should we expose this capability in package:objective_c? It might be useful to some users, but it's a bit problematic.

In a language like C++ we could make a util class that calls push in the constructor and pop in the destructor, and users could declare a local variable of this class to turn their scope into an autorelease pool.

But in Dart if we did that using finalizers, pop would be called at an undefined time in the future. Worse, this could mean pools are popped out of order (popping an outer pool pops all the inner pools, so later when the inner pool is popped we'd be double popping, which is not allowed).

So our only option would be to expose push and pop directly as standalone functions, which is a bit of a footgun.

dcharkes commented 1 month ago

How about using the scoping structure like we do with Arena and using?

autoReleasePool(() {
  // Do stuff...
});
liamappelbe commented 1 week ago

While working on https://github.com/dart-lang/native/pull/1573 I ran into some issues with autorelease pool reliability that makes me think we should be very careful about how we expose them to users, if at all. Judging by the problems I was seeing, I think flutter uses autorelease pools internally. That's fine, except that it's really easy to accidentally interleave your own pools with theirs, especially if you're working with async code, leading to those double popping issues I mentioned.

I think the only safe API is Daco's suggestion, with the added restriction that the function must be sync. That is, we'll need to ensure (either in the type system or with a runtime type check) that the result of the callback is not a Future.