sql.js icon indicating copy to clipboard operation
sql.js copied to clipboard

When calling functions on a closed database, confusing "out of memory" error is reported

Open Taytay opened this issue 7 years ago • 8 comments

I realize that calling methods on a database that's been closed is not a good idea. However, when calling a method like each on the closed database, the error reported is out of memory. The underlying db is nulled out on close (which makes sense). But then, in the handleError method:

Database.prototype.handleError = function(returnCode) {
    var errmsg;
    if (returnCode === SQLite.OK) {
      return null;
    } else {
      errmsg = sqlite3_errmsg(this.db);
      throw new Error(errmsg);
    }
  };

We're passing null into the sqlite3_errmsg. That in turn causes it to report it as a memory allocation error.

Ideally, the error reported would be something more intuitive. I bring this up because I am now closing databases that we are no longer using in an effort to fix a real memory allocation error. However, in the process of doing so, I accidentally closed a database I shouldn't have and continued to get what looked like memory errors. It wasn't until later that I realized this message was inaccurate.

Taytay avatar Jun 12 '17 21:06 Taytay

This is clearly a bug. Thank you for reporting it! Would you mind making a pull request for this ?

lovasoa avatar Jun 12 '17 22:06 lovasoa

I'd be happy to, but I wasn't sure what the ideal fix was. I presume we still need to null out the DB, so that leaves two options: 1) If we pass in a null DB to the handleError function, we can assume that means that the DB has been closed. 2) We can proactively check for, and throw errors if you make any method call on a closed db. However, both of these diverge from SQLite's original behavior somewhat. Ideally we'd report the exact same error code/message it reports. However, without the existing DB, handleError doesn't know what the error code would be. Is there a precedent for handling this? If not, I'll just give it my best :)

Taytay avatar Jun 12 '17 23:06 Taytay

Can someone fix this?

This incorrect error is incredibly strange behavior. I'm very new to SQL.js so I can't make the PR myself, but if someone could make the PR, I think it would be really helpful. It's just such a strange error to see. I spent hours looking (or re-remembering) how to manually garbage collect memory in NodeJS. So there are probably many people are spending a lot of time looking at the memory issue when it's really just this red-herring error.

dgreene1 avatar Aug 06 '19 03:08 dgreene1

I'm also facing a out of memory error and have no idea what could be wrong. I'm trying to do:

const blob: Uint8Array = ...
new SQL.Database(blob);

Any idea how to debug this?

fdietze avatar Dec 20 '20 19:12 fdietze

The most common cause for out of memory errors, is probably... being out of memory. Isn't your blob just too large ?

lovasoa avatar Dec 20 '20 20:12 lovasoa

It's only 64K. It worked before. And out of a sudden I got this error. I even checked out an older revision on the same project where I knew it was working. And the same error is appearing. Very strange.

fdietze avatar Dec 20 '20 21:12 fdietze

So I tried again.

I successfully lodaed my database file in the online demo (https://sql.js.org/examples/GUI/) and everything is working well.

Now, I'm using this code:

  let blob = new Uint8Array(await (await fetch(url)).arrayBuffer());
  console.log(blob);
  new SQL.Database(blob);

And in the console I get:

Uint8Array(110592) [83, 81, 76, 105, …]
sql-wasm.min.js:1 Uncaught (in promise) Error: out of memory
    at e.handleError (sql-wasm.min.js:1)
    at new e (sql-wasm.min.js:1)
    at loadCachedDb (modelstorage.ts:28) // the line with `new SQL.Database(blob);`

Any ideas what could be wrong? I'm blocked.

fdietze avatar Dec 27 '20 15:12 fdietze

I solved it. It seems like the hosted wasm binary has changed. This fixed my issue:

  const SQL = await initSqlJs({
-    locateFile: (file) => `https://sql.js.org/dist/${file}`,
+    locateFile: (file) => `https://cdnjs.cloudflare.com/ajax/libs/sql.js/1.4.0/dist/${file}`,
  });

fdietze avatar Dec 27 '20 16:12 fdietze