Database not properly closed when .close() is called while in "busy" state

[rooflack]

Hi @WebReflection,

I think I may have found a little bug in the current version of the library. This is related to the close() method: if you call this method while the object is marked as busy, then the ".exit" command is queued but never gets run, because the notWorking flag is set immediately, and then when query() is called again by next(), the method exits before actually running the command.

This sequence of events can happen for instance if you run a select query that raises a fatal error (that makes sqlite output to stderr but not to stdout, so the busy flag is not reset), and then try to close the database.

As a (rather ugly) workaround, setting ignoreErrors==true and adding a call to a successful select before closing solves the problem. E.g.:

db.ignoreErrors= true;
db.query("select 1", function(err, data) {
    db.close();
});

(This "resets" the busy state before the call to close(), so that .exit will be properly run regardless of what happened before).

This is quite a minor issue, as the object is unusable anyway after the call to close(), but it can interfere when doing unit tests with lots of sequential database openings and closings, as the db file stays open (thus locked) until the dblite object gets garbage-collected, instead of being explicitly freed when close() is called.

Please let me know if you need more information to reproduce this. Bye!

[WebReflection]

this is a very valid concern and I'll try to wrap my head around a better solution. Thanks

[WebReflection]

can you please tell me if using this login instead all works as expected?

  if (
      // notWorking is set once .close() has been called
      // it does not make sense to execute anything after
      // the program is being closed
      notWorking &&
      // however, since at that time the program could also be busy
      // let's be sure that this is either the exit call
      // or that the last call is still the exit one
      !(string === '.exit' || queue[queue.length - 1][0] === '.exit')
    ) return onerror('closing'), self;

This would be the very beginning of method self.query = function(string, params, fields, callback)

Thanks a lot

[rooflack]

Hello,

I just tested this on the version I'm currently using (0.6.1), and it looks like it does the job: all the unit tests that used to timeout with the original code now pass properly.

Thanks a lot :-)

Cheers!

[WebReflection]

thanks for testing, it's up and running on 0.7.1