Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ObjCBlock.fromFunction(closure) indefinitely hangs on to closures and therefore leaks memory #225

Closed
mkustermann opened this issue Oct 7, 2022 · 1 comment

Comments

@mkustermann
Copy link
Member

Right now the ObjCBlock.fromFunction(closure) is implemented as follows:

final _ObjCBlock_closureRegistry = <int, Function>{};
int _ObjCBlock_closureRegistryIndex = 0;

ffi.Pointer<ffi.Void> _ObjCBlock_registerClosure(Function fn) {
  final id = ++_ObjCBlock_closureRegistryIndex;
  _ObjCBlock_closureRegistry[id] = fn;
  return ffi.Pointer<ffi.Void>.fromAddress(id);
}

void _ObjCBlock_closureTrampoline(
    ffi.Pointer<_ObjCBlock> block, int arg0, ffi.Pointer<ffi.Bool> arg1) {
  return _ObjCBlock_closureRegistry[block.ref.target.address]!(arg0, arg1);
}

class ObjCBlock extends _ObjCBlockBase {
  ...
  /// Creates a block from a Dart function.
  ObjCBlock.fromFunction(
      AVFAudio lib, void Function(int arg0, ffi.Pointer<ffi.Bool> arg1) fn) 
      : this._(
            lib._newBlock1(
                ffi.Pointer.fromFunction<
                            ffi.Void Function(ffi.Pointer<_ObjCBlock> block,
                                NSUInteger arg0, ffi.Pointer<ffi.Bool> arg1)>(
                        _ObjCBlock_closureTrampoline)
                    .cast(),
                _ObjCBlock_registerClosure(fn)),
            lib);
  ...
}

This registers the closure in a global map where it's never removed from again - effectively creating a memory leak. Since the closures that are passed to ObjCBlock.fromFunction(closure) may hang on to non trivial amounts of memory, this seems like a big problem.

If memory is managed manually, it would mean calling ObjCBlock.release() should also release the closure from the map.

@liamappelbe
Copy link
Contributor

Duplicate of #204

@liamappelbe liamappelbe transferred this issue from dart-archive/ffigen Nov 15, 2023
HosseinYousefi pushed a commit that referenced this issue Nov 16, 2023
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.3.0 to 3.5.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@ac59398...8f4b7f8)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
HosseinYousefi pushed a commit that referenced this issue Nov 16, 2023
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.3.0 to 3.5.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@ac59398...8f4b7f8)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants