NoteDbMigrator: Actually close thread-local cached open ReviewDb

In Ia2b77fdc we tried to be clever and override the close method of the
ReviewDb that is shared across a single thread, deferring the close
until ContextHelper#close. Unfortunately, we just called the no-op close
method, rather than the close method of the underlying db, resulting in
leaking a DB handle after running NoteDbMigrator.

This caused StandaloneNoteDbMigrationIT to fail with:

  Caused by: org.h2.jdbc.JdbcSQLException: Database may be already in use: "Locked by another process". Possible solutions: close all other connection(s); use the server mode [90020-176]

Surprisingly, this doesn't affect the tests when run in the Bazel
sandbox, only in Eclipse. I don't have an explanation for this other
than that there might be some different filesystem semantics in the
sandbox that affect H2's locking protocol. Unfortunately, I was also not
able to reproduce the failure by adding "local" to the tags of the pgm
acceptance test target, so maybe the sandbox is not the problem after
all.

Nonetheless, this was clearly a bug, and this change causes tests to
pass in Eclipse.

Change-Id: I27ec98ce99fc3af082572bed0ea27b3eb3f2c3ee
This commit is contained in:
Dave Borowitz
2017-07-12 12:18:57 -04:00
parent 2457718658
commit 43b74a28aa

View File

@@ -744,6 +744,7 @@ public class NoteDbMigrator implements AutoCloseable {
private class ContextHelper implements AutoCloseable {
private final Thread callingThread;
private ReviewDb db;
private Runnable closeDb;
ContextHelper() {
callingThread = Thread.currentThread();
@@ -760,8 +761,10 @@ public class NoteDbMigrator implements AutoCloseable {
synchronized ReviewDb getReviewDb() throws OrmException {
if (db == null) {
ReviewDb actual = schemaFactory.open();
closeDb = actual::close;
db =
new ReviewDbWrapper(unwrapDb(schemaFactory.open())) {
new ReviewDbWrapper(unwrapDb(actual)) {
@Override
public void close() {
// Closed by ContextHelper#close.
@@ -774,7 +777,9 @@ public class NoteDbMigrator implements AutoCloseable {
@Override
public synchronized void close() {
if (db != null) {
db.close();
closeDb.run();
db = null;
closeDb = null;
}
}
}