dart-lang / native

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

App is crashing after trying to free reassigned pointer #927

Open kirill-21 opened 1 year ago

kirill-21 commented 1 year ago

This is the place where crash happens, if you remove this line then there will be no crash. And if this pointer is not freed then it leads to a huge memory leak

  // CRASH IS HAPPENING HERE:
  calloc.free(buffer);
  ffi: ^2.1.0
  win32: ^5.0.7
import 'dart:ffi';

import 'package:ffi/ffi.dart';
import 'package:flutter/material.dart';
import 'package:win32/win32.dart';

/// Loops through all system processes with [NtQuerySystemInformation].
Future<bool> loopThroughSystemProcessesWithNtApi(Function callback) async {
  final Pointer<Uint32> dwSize = calloc<Uint32>();
  Pointer<SystemProcessInformation> buffer = calloc<SystemProcessInformation>();

  // SystemProcessInformation = 5;
  int info = dNtQuerySystemInformation(5, nullptr, 0, dwSize);

  // [STATUS_INFO_LENGTH_MISMATCH] means that we've got the size and now need
  // to call this method once again but with actual pointer to the buffer
  if (!ntSuccess(info) && info == statusInfoLengthMismatch) {
    calloc.free(buffer);
    buffer = calloc<SystemProcessInformation>(dwSize.value);
    info = dNtQuerySystemInformation(5, buffer, dwSize.value, dwSize);
  }

  // Failed to retrieve processes list
  if (!ntSuccess(info)) {
    calloc.free(dwSize);
    calloc.free(buffer);
    return false;
  }

  // Loop through all found entities
  while (buffer.ref.nextEntryOffset != 0) {
    try {
      // Stop looping if we found what we've been looking for
      final bool result = await callback.call(buffer) as bool;
      if (result) break;

      // Stop updating the loop to avoid crashes
      if (buffer.ref.nextEntryOffset == 0) break;

      // Update buffer to the next process
      buffer = buffer
          .cast<Uint8>()
          .elementAt(buffer.ref.nextEntryOffset)
          .cast<SystemProcessInformation>();
    } catch (_) {
      break;
    }
  }

  // CRASH IS HAPPENING HERE:
  calloc.free(buffer);

  calloc.free(dwSize);
  return true;
}

/// Macros for NTSTATUS.
const int statusInfoLengthMismatch = -1073741820;

// Shortcut for Pointer<Uint32>
typedef PULONG = Pointer<Uint32>;

/// Checks if Nt... function has succeded.
bool ntSuccess(int result) => result >= 0;

// Allocalte ntdll API
final Pointer<Utf16> _ntdllTitle = 'ntdll.dll'.toNativeUtf16();
final int _ntdllHndle = GetModuleHandle(_ntdllTitle);

/// Dart representation of native [NtQuerySystemInformation] function.
/// ```c
/// [in]            SYSTEM_INFORMATION_CLASS SystemInformationClass,
/// [in, out]       PVOID                    SystemInformation,
/// [in]            ULONG                    SystemInformationLength,
/// [out, optional] PULONG                   ReturnLength
/// ```
int Function(int, Pointer<NativeType>, int, PULONG)?
    _pNtQuerySystemInformationFunc;

/// [NtQuerySystemInformation may be altered or unavailable in future versions
/// of Windows. Applications should use the alternate functions
/// listed in this topic.] Retrieves the specified system information.
int dNtQuerySystemInformation(
  int dwFlag,
  Pointer<NativeType> buffer,
  int dwSize,
  PULONG out,
) {
  if (_pNtQuerySystemInformationFunc == null) {
    // Find function address with
    final Pointer<Utf8> ansi = 'NtQuerySystemInformation'.toANSI();
    final Pointer pNtQuerySystemInformation = GetProcAddress(_ntdllHndle, ansi);
    calloc.free(ansi);

    // Return 0 if function was not found
    if (pNtQuerySystemInformation == nullptr) return 0;

    _pNtQuerySystemInformationFunc = pNtQuerySystemInformation
        .cast<NativeFunction<NTSTATUS Function(ULONG, PVOID, ULONG, PULONG)>>()
        .asFunction<int Function(int, PVOID, int, PULONG)>();
  }

  return _pNtQuerySystemInformationFunc?.call(dwFlag, buffer, dwSize, out) ?? 0;
}

/// Basic info about the process
sealed class SystemProcessInformation extends Struct {
  @Uint32()
  external int nextEntryOffset;
  @Uint32()
  external int numberOfThreads;
  external LargeInteger workingSetPrivateSize;
  @Uint32()
  external int hardFaultCount;
  @Uint32()
  external int numberOfThreadsHighWatermark;
  @Uint64()
  external int cycleTime;
  external LargeInteger createTime;
  external LargeInteger userTime;
  external LargeInteger kernelTime;
  external UnicodeString imageName;
  @Int32()
  external int basePriority;
  @IntPtr()
  external int uniqueProcessId;
  @IntPtr()
  external int inheritedFromUniqueProcessId;
  @Uint32()
  external int handleCount;
  @Uint32()
  external int sessionId;
  @IntPtr()
  external int uniqueProcessKey;
  @IntPtr()
  external int peakVirtualSize;
  @IntPtr()
  external int virtualSize;
  @Uint32()
  external int pageFaultCount;
  @IntPtr()
  external int peakWorkingSetSize;
  @IntPtr()
  external int workingSetSize;
  @IntPtr()
  external int quotaPeakPagedPoolUsage;
  @IntPtr()
  external int quotaPagedPoolUsage;
  @IntPtr()
  external int quotaPeakNonPagedPoolUsage;
  @IntPtr()
  external int quotaNonPagedPoolUsage;
  @IntPtr()
  external int pagefileUsage;
  @IntPtr()
  external int peakPagefileUsage;
  @IntPtr()
  external int ppivatePageCount;
  external LargeInteger readOperationCount;
  external LargeInteger writeOperationCount;
  external LargeInteger otherOperationCount;
  external LargeInteger readTransferCount;
  external LargeInteger writeTransferCount;
  external LargeInteger otherTransferCount;
  @Array(1)
  external Array<SystemThreadInformation> threads;
}

/// Represents a 64-bit signed integer value.
sealed class LargeInteger extends Union {
  external _LargeIntegerFirst firstStruct;
  external _LargeIntegerSecond secondStruct;
  @Int64()
  external int quadPart;
}

/// Structure for LargeInteger union.
sealed class _LargeIntegerFirst extends Struct {
  @Uint32()
  external int lowPart;
  @Int32()
  external int highPart;
}

/// Structure for LargeInteger union.
sealed class _LargeIntegerSecond extends Struct {
  @Uint32()
  external int lowPart;
  @Int32()
  external int highPart;
}

/// The SYSTEM_THREAD_INFORMATION structure contains information about
/// a thread running on a system.
sealed class SystemThreadInformation extends Struct {
  external LargeInteger kernelTime;
  external LargeInteger userTime;
  external LargeInteger createTime;
  @Uint32()
  external int waitTime;
  external Pointer<Void> startAddress;
  external ClientID clientId;
  @Int32()
  external int priority;
  @Int32()
  external int basePriority;
  @Uint32()
  external int contextSwitches;
  @Uint32()
  external int threadState;
  @Uint32()
  external int waitReason;
}

/// The UNICODE_STRING structure is used to define Unicode strings.
sealed class ClientID extends Struct {
  @IntPtr()
  external int uniqueProcess;
  @IntPtr()
  external int uniqueThread;
}

/// The UNICODE_STRING structure is used to define Unicode strings.
sealed class UnicodeString extends Struct {
  @Uint16()
  external int length;
  @Uint16()
  external int maximumLength;
  external Pointer<Utf16> buffer;
}

void main() {
  loopThroughSystemProcessesWithNtApi(
    (Pointer<SystemProcessInformation> info) {
      print('123');
      return false;
    },
  );
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        // This is the theme of your application.
        //
        // TRY THIS: Try running your application with "flutter run". You'll see
        // the application has a blue toolbar. Then, without quitting the app,
        // try changing the seedColor in the colorScheme below to Colors.green
        // and then invoke "hot reload" (save your changes or press the "hot
        // reload" button in a Flutter-supported IDE, or press "r" if you used
        // the command line to start the app).
        //
        // Notice that the counter didn't reset back to zero; the application
        // state is not lost during the reload. To reset the state, use hot
        // restart instead.
        //
        // This works for code too, not just values: Most code changes can be
        // tested with just a hot reload.
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: const MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});

  // This widget is the home page of your application. It is stateful, meaning
  // that it has a State object (defined below) that contains fields that affect
  // how it looks.

  // This class is the configuration for the state. It holds the values (in this
  // case the title) provided by the parent (in this case the App widget) and
  // used by the build method of the State. Fields in a Widget subclass are
  // always marked "final".

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  int _counter = 0;

  void _incrementCounter() {
    setState(() {
      // This call to setState tells the Flutter framework that something has
      // changed in this State, which causes it to rerun the build method below
      // so that the display can reflect the updated values. If we changed
      // _counter without calling setState(), then the build method would not be
      // called again, and so nothing would appear to happen.
      _counter++;
    });
  }

  @override
  Widget build(BuildContext context) {
    // This method is rerun every time setState is called, for instance as done
    // by the _incrementCounter method above.
    //
    // The Flutter framework has been optimized to make rerunning build methods
    // fast, so that you can just rebuild anything that needs updating rather
    // than having to individually change instances of widgets.
    return Scaffold(
      appBar: AppBar(
        // TRY THIS: Try changing the color here to a specific color (to
        // Colors.amber, perhaps?) and trigger a hot reload to see the AppBar
        // change color while the other colors stay the same.
        backgroundColor: Theme.of(context).colorScheme.inversePrimary,
        // Here we take the value from the MyHomePage object that was created by
        // the App.build method, and use it to set our appbar title.
        title: Text(widget.title),
      ),
      body: Center(
        // Center is a layout widget. It takes a single child and positions it
        // in the middle of the parent.
        child: Column(
          // Column is also a layout widget. It takes a list of children and
          // arranges them vertically. By default, it sizes itself to fit its
          // children horizontally, and tries to be as tall as its parent.
          //
          // Column has various properties to control how it sizes itself and
          // how it positions its children. Here we use mainAxisAlignment to
          // center the children vertically; the main axis here is the vertical
          // axis because Columns are vertical (the cross axis would be
          // horizontal).
          //
          // TRY THIS: Invoke "debug painting" (choose the "Toggle Debug Paint"
          // action in the IDE, or press "p" in the console), to see the
          // wireframe for each widget.
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            const Text(
              'You have pushed the button this many times:',
            ),
            Text(
              '$_counter',
              style: Theme.of(context).textTheme.headlineMedium,
            ),
          ],
        ),
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: _incrementCounter,
        tooltip: 'Increment',
        child: const Icon(Icons.add),
      ), // This trailing comma makes auto-formatting nicer for build methods.
    );
  }
}
dcharkes commented 1 year ago

You're trying to free a different memory address than you allocate in this code.

Allocation:

  Pointer<SystemProcessInformation> buffer = calloc<SystemProcessInformation>();

Or allocation:

    calloc.free(buffer);
    buffer = calloc<SystemProcessInformation>(dwSize.value);

Moving the pointer forward:

      buffer = buffer
          .cast<Uint8>()
          .elementAt(buffer.ref.nextEntryOffset)
          .cast<SystemProcessInformation>();

The pointer is now no longer the same pointer.

Freeing with a memory address that is somewhere inside the allocated buffer rather than at the beginning:

  calloc.free(buffer);

I believe you should free with the original pointer, not with the one that is moved.

kirill-21 commented 1 year ago

You're trying to free a different memory address than you allocate in this code.

Allocation:

  Pointer<SystemProcessInformation> buffer = calloc<SystemProcessInformation>();

Or allocation:

    calloc.free(buffer);
    buffer = calloc<SystemProcessInformation>(dwSize.value);

Moving the pointer forward:

      buffer = buffer
          .cast<Uint8>()
          .elementAt(buffer.ref.nextEntryOffset)
          .cast<SystemProcessInformation>();

The pointer is now no longer the same pointer.

Freeing with a memory address that is somewhere inside the allocated buffer rather than at the beginning:

  calloc.free(buffer);

I believe you should free with the original pointer, not with the one that is moved.

How do I do that? Can you test this code?

I tried to do this:

final int realBufferAddress = buffer.address;

// loop

buffer = Pointer.fromAddress(realBufferAddress); calloc.free(buffer);

This does not lead to a crash, however, according to memory view in taskmanager it's not released

image

So freeing with crash frees the memory and freeing without doesn't :c

kirill-21 commented 1 year ago

Situation seems to be much better with malloc, memory is much more efficient , however, this still does not fully free the memory

dcharkes commented 1 year ago

buffer = Pointer.fromAddress(realBufferAddress); calloc.free(buffer);

👍

This does not lead to a crash, however, according to memory view in taskmanager it's not released

It should be, maybe there's another issue with the code? Are you allocating somewhere else?

Situation seems to be much better with malloc, memory is much more efficient , however, this still does not fully free the memory

Malloc should maybe be faster because the memory does not have to be zeroed out, but I'm not sure why it would lead to less memory usage.

Just looking at the task manager does not tell you what is holding on to the memory. Is it the Dart heap? Is it non-freed C memory? You could try using the memory view in devtools: https://docs.flutter.dev/tools/devtools/memory

kirill-21 commented 1 year ago

buffer = Pointer.fromAddress(realBufferAddress); calloc.free(buffer);

👍

This does not lead to a crash, however, according to memory view in taskmanager it's not released

It should be, maybe there's another issue with the code? Are you allocating somewhere else?

Situation seems to be much better with malloc, memory is much more efficient , however, this still does not fully free the memory

Malloc should maybe be faster because the memory does not have to be zeroed out, but I'm not sure why it would lead to less memory usage.

Just looking at the task manager does not tell you what is holding on to the memory. Is it the Dart heap? Is it non-freed C memory? You could try using the memory view in devtools: https://docs.flutter.dev/tools/devtools/memory

I see that with calloc my process can go up to 5GB of memory usage, with malloc it's only 350MB. (Can be tested if looping this code in long while loop)

No, I'm testing it in the example I've provided, memory is not allocated anywhere else. I'll check devtools in the morning

kirill-21 commented 1 year ago

Just looking at the task manager does not tell you what is holding on to the memory. Is it the Dart heap? Is it non-freed C memory? You could try using the memory view in devtools: https://docs.flutter.dev/tools/devtools/memory

image

dcharkes commented 1 year ago

Interesting, 4 GB of RSS (resident set size). That could be the C memory. @rmacnak-google @bkonyi does FFI malloced memory show up in the RSS? I presume the "Dart/Flutter Native" only contains the runtime allocated memory, correct?

You could try wrapping your malloc and free calls to log which ones you are freeing. It seems very strange that malloc and calloc from package:ffi would have different behavior in your application. On windows, they are backed by the same native allocator winCoTaskMemAlloc, so it's really unlikely that it would behave differently on the native side.

rmacnak-google commented 1 year ago

Linux: Usually malloc'd memory will be in RSS. Memory is part of RSS once it's been touched. Often a small allocation will be on the same page as malloc metadata or a small calloc allocation will be explicitly zeroed. But potentially a large malloc or calloc allocation gets a mmapped region to itself, and doesn't count toward RSS because it will be lazily paged in with zeroed pages.

Windows: I'm not sure whether the pages are brought in lazily or eagerly.

dcharkes commented 1 year ago

On Windows, we zero out the memory manually (CoTaskMemAlloc doesn't have a zeroed out version), so that could move the memory in to the RSS if it works similarly on Windows.

@kirill-21 it might be that you're leaking native memory in your application forgetting to free, but that with malloc from package:ffi it shows up less aggressively than with calloc due to the above.

You could try wrapping your malloc and free calls to log which ones you are freeing.

This is your best bet to figuring out what's going on.

kirill-21 commented 1 year ago

On Windows, we zero out the memory manually (CoTaskMemAlloc doesn't have a zeroed out version), so that could move the memory in to the RSS if it works similarly on Windows.

@kirill-21 it might be that you're leaking native memory in your application forgetting to free, but that with malloc from package:ffi it shows up less aggressively than with calloc due to the above.

You could try wrapping your malloc and free calls to log which ones you are freeing.

This is your best bet to figuring out what's going on.

If I free it my app crashes, if I free it with original pointer address then not all memory is released. Because when I do it with crashing and I can see in memory usage that it drops to ~200MB. With non-crash approach this value is at least twice bigger (with calloc)