Memory leak on execute for allocated Arguments

[BrendanLacey]

When inserting large amounts of objects, ram usage grows out of control. Attached is an example inserting 250k records and after everything is disposed of, ram usage stays 400 mb higher than starting. This is causing out of memory crashes on Android when attempting to data load records.

Upon researching this, deallocateArguments() in FfiStatmennt doesn't seem to be called.
The only place I can find referencing it is inside FinalizableStatment, and it has a check for _hasAllocatedArguments beforehand, but there is nothing in the code to ever set to that to be true

void _deallocateArguments() {
    if (_hasAllocatedArguments) {
      _hasAllocatedArguments = false;
      statement.deallocateArguments();
    }
  }

To further test, I created a fork and forced a call to deallocateArguments and it has fixed the memory leak locally for me, however I don't think I am doing that in the correct place.

Thanks, let me know if there are further things I can provide

Code to reproduce:

final dbFolder = await getApplicationCacheDirectory();
    var db = sqlite3.open(p.join(dbFolder.path, 'db.sqlite'));
    
    db.execute("CREATE TABLE IF NOT EXISTS \"large_data_test_table\" (\"id\" INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, \"title\" TEXT NOT NULL, \"body\" TEXT NOT NULL, \"category\" INTEGER NULL, \"text_column1\" TEXT NULL, \"text_column2\" TEXT NULL, \"text_column3\" TEXT NULL, \"text_column4\" TEXT NULL, \"text_column5\" TEXT NULL, \"text_column6\" TEXT NULL, \"text_column7\" TEXT NULL, \"text_column8\" TEXT NULL, \"text_column9\" TEXT NULL, \"text_column10\" TEXT NULL, \"text_column11\" TEXT NULL, \"text_column12\" TEXT NULL, \"text_column13\" TEXT NULL, \"text_column14\" TEXT NULL, \"text_column15\" TEXT NULL, \"text_column16\" TEXT NULL, \"text_column17\" TEXT NULL, \"text_column18\" TEXT NULL, \"text_column19\" TEXT NULL, \"text_column20\" TEXT NULL, \"text_column21\" TEXT NULL, \"text_column22\" TEXT NULL, \"text_column23\" TEXT NULL, \"text_column24\" TEXT NULL, \"text_column25\" TEXT NULL, \"text_column26\" TEXT NULL, \"text_column27\" TEXT NULL, \"text_column28\" TEXT NULL, \"text_column29\" TEXT NULL, \"text_column30\" TEXT NULL, \"text_column31\" TEXT NULL, \"text_column32\" TEXT NULL, \"text_column33\" TEXT NULL, \"text_column34\" TEXT NULL, \"text_column35\" TEXT NULL, \"text_column36\" TEXT NULL, \"text_column37\" TEXT NULL, \"text_column38\" TEXT NULL, \"text_column39\" TEXT NULL, \"text_column40\" TEXT NULL, \"text_column41\" TEXT NULL, \"text_column42\" TEXT NULL, \"text_column43\" TEXT NULL, \"text_column44\" TEXT NULL, \"text_column45\" TEXT NULL, \"text_column46\" TEXT NULL, \"text_column47\" TEXT NULL, \"text_column48\" TEXT NULL, \"text_column49\" TEXT NULL, \"text_column50\" TEXT NULL);");

    var prepared = db.prepare("INSERT INTO large_data_test_table"
      " (title, body, text_column1, text_column2, text_column3, text_column4, text_column5, text_column6, text_column7, text_column8, text_column9, text_column10,"
      " text_column11, text_column12, text_column13, text_column14, text_column15, text_column16, text_column17, text_column18, text_column19, text_column20,"
      " text_column21, text_column22, text_column23, text_column24, text_column25, text_column26, text_column27, text_column28, text_column29, text_column30,"
      " text_column31, text_column32, text_column33, text_column34, text_column35, text_column36, text_column37, text_column38, text_column39, text_column40,"
      " text_column41, text_column42, text_column43, text_column44, text_column45, text_column46, text_column47, text_column48, text_column49, text_column50)"
      " VALUES "
      "(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);");

    for (int i = 0; i < 25; i++){
      db.execute("BEGIN TRANSACTION");
      for (int n = 0; n < 10000; n++){
        prepared.execute([
          "Title: ${"Some Value 1234"}","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234",
        "Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234",
        "Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234",
        "Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234",
        "Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234","Some Value 1234",
        "Some Value 1234","Some Value 1234",
        ]);
      }
      db.execute("COMMIT");
    }

    prepared.dispose();
    db.dispose();

[simolus3]

Argh!! The _hasAllocatedArguments thing was probably a leftover from an earlier version, and I have forgotten about it during the refactor to share more code between wasm/ffi.

Looking at both underlying implementations which keep a list of allocated pointers and clear them after freeing them, there's no double-free risk with just removing that guard. Also, _deallocateArguments is only called in the Dart finalizer and in _reset(), which in turn only gets called immediately before binding new parameters. So there's no scenario in which the arguments could still be in use by sqlite3. So, I'll just remove the _hasAllocatedArguments flag entirely.

[simolus3]

Fixed in 2.4.2. Thank you very much for the detailed report and analysis!

[BrendanLacey]

Thank you, incredible speed with this. I'm used to large companies where it takes months to even get a bug report reviewed.