Skip to content

Commit 8e8e229

Browse files
committed
fix: pin AutoNumber tables to survive GC between DDL and DML
Jackcess's internal table cache holds Table instances only via WeakReference. DISABLE/ENABLE AUTOINCREMENT ON toggles allowAutoNumberInsert directly on the Table object returned by getTable(), but nothing kept a strong reference to it afterwards. If GC collected that table between the DDL statement and a subsequent INSERT, the table was transparently reloaded with the flag reset to its default, silently discarding explicit AutoNumber values in favor of auto-generated ones. DBReference now pins tables with an explicit allowAutoNumberInsert toggle in a strongly-referenced map, cleared whenever dbIO is reloaded. Add a regression test that forces GC passes between the DDL and DML statements to catch this deterministically.
1 parent 91300c1 commit 8e8e229

4 files changed

Lines changed: 65 additions & 1 deletion

File tree

src/main/java/net/ucanaccess/jdbc/AbstractExecute.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package net.ucanaccess.jdbc;
22

3+
import io.github.spannm.jackcess.Table;
34
import net.ucanaccess.commands.DDLCommandEnlist;
45
import net.ucanaccess.converters.Metadata;
56
import net.ucanaccess.converters.SQLConverter;
@@ -65,7 +66,11 @@ private Object enableDisable(DDLType ddlType) throws SQLException, IOException {
6566
throw new TableNotFoundException(tableName);
6667
}
6768
boolean enableAutoIncr = ddlType.equals(DDLType.ENABLE_AUTOINCREMENT);
68-
conn.getDbIO().getTable(rtn).setAllowAutoNumberInsert(!enableAutoIncr);
69+
Table table = conn.getDbIO().getTable(rtn);
70+
table.setAllowAutoNumberInsert(!enableAutoIncr);
71+
// Jackcess only holds this Table via a WeakReference in its internal cache; pin it here so a GC pass
72+
// between this DDL statement and a later DML statement can't silently reset the flag we just set.
73+
conn.pinAutoNumberTable(rtn, table);
6974
if (this instanceof Execute) {
7075
return false;
7176
} else {

src/main/java/net/ucanaccess/jdbc/DBReference.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.ArrayList;
3030
import java.util.Arrays;
3131
import java.util.Date;
32+
import java.util.HashMap;
3233
import java.util.HashSet;
3334
import java.util.List;
3435
import java.util.Map;
@@ -72,6 +73,14 @@ public class DBReference {
7273
private boolean hsqldbShutdown;
7374
private File mirrorFolder;
7475
private final Set<File> links = new HashSet<>();
76+
/**
77+
* Holds strong references to {@link Table} instances on which a caller has explicitly toggled
78+
* {@code allowAutoNumberInsert} (via {@code DISABLE}/{@code ENABLE AUTOINCREMENT ON}). Jackcess's internal
79+
* table cache only holds {@link Table} objects via {@code WeakReference}s, so without this pin the GC could
80+
* collect such a table between statements; the table would then be transparently reloaded on next access,
81+
* silently resetting {@code allowAutoNumberInsert} to its default and losing the toggle.
82+
*/
83+
private final Map<String, Table> pinnedAutoNumberTables = new HashMap<>();
7584
private boolean ignoreCase = true;
7685
private boolean mirrorReadOnly;
7786
private Integer lobScale;
@@ -214,6 +223,7 @@ Connection checkLastModified(Connection conn, Session session) throws Ucanaccess
214223
closeHsqlDb(session);
215224
dbIO.flush();
216225
dbIO.close();
226+
pinnedAutoNumberTables.clear();
217227
dbIO = open(dbFile, pwd);
218228
id = createId();
219229
firstConnection = true;
@@ -558,10 +568,19 @@ public void reloadDbIO() throws IOException {
558568
for (IOnReloadReferenceListener listener : onReloadListeners) {
559569
listener.onReload();
560570
}
571+
pinnedAutoNumberTables.clear();
561572
dbIO = open(dbFile, pwd);
562573

563574
}
564575

576+
/**
577+
* Pins the given table so it is not garbage-collected while an explicit {@code allowAutoNumberInsert} toggle
578+
* (from {@code DISABLE}/{@code ENABLE AUTOINCREMENT ON}) is in effect on it.
579+
*/
580+
void pinAutoNumberTable(String tableName, Table table) {
581+
pinnedAutoNumberTables.put(tableName, table);
582+
}
583+
565584
public void setInactivityTimeout(int inactivityTimeout) {
566585
memoryTimer.setInactivityTimeout(inactivityTimeout);
567586
}
@@ -586,6 +605,7 @@ void shutdown(Session session) throws Exception {
586605
}
587606
}
588607
memoryTimer.timer.cancel();
608+
pinnedAutoNumberTables.clear();
589609
dbIO.flush();
590610
dbIO.close();
591611
closeHsqlDb(session);

src/main/java/net/ucanaccess/jdbc/UcanaccessConnection.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package net.ucanaccess.jdbc;
22

33
import io.github.spannm.jackcess.Database;
4+
import io.github.spannm.jackcess.Table;
45
import net.ucanaccess.commands.CompositeCommand;
56
import net.ucanaccess.commands.ICommand;
67
import net.ucanaccess.commands.ICommand.CommandType;
@@ -350,6 +351,10 @@ public Database getDbIO() {
350351
return ref.getDbIO();
351352
}
352353

354+
void pinAutoNumberTable(String tableName, Table table) {
355+
ref.pinAutoNumberTable(tableName, table);
356+
}
357+
353358
@Override
354359
public int getHoldability() throws SQLException {
355360
try {

src/test/java/net/ucanaccess/jdbc/CounterTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,38 @@ void testCreateTypes(AccessVersion accessVersion) throws SQLException, IOExcepti
6161

6262
}
6363

64+
/**
65+
* Regression test for a bug where Jackcess's internal table cache holds {@link io.github.spannm.jackcess.Table}
66+
* instances only via {@code WeakReference}. Without pinning, a GC pass between the {@code DISABLE AUTOINCREMENT
67+
* ON} DDL statement and a subsequent {@code INSERT} could collect the {@code Table} object whose
68+
* {@code allowAutoNumberInsert} flag had just been toggled; the table would then be silently reloaded with the
69+
* flag reset to its default, and an explicit AutoNumber value would be replaced by an auto-generated one.
70+
*/
71+
@ParameterizedTest(name = "[{index}] {0}")
72+
@AccessVersionSource(include = "V2016")
73+
void testDisableAutoincrementSurvivesGc(AccessVersion accessVersion) throws SQLException, IOException {
74+
init(accessVersion);
75+
76+
try (UcanaccessStatement st = ucanaccess.createStatement()) {
77+
executeStatements(st, "DISABLE AUTOINCREMENT ON t_counter");
78+
assertTrue(st.getConnection().getDbIO().getTable("t_counter").isAllowAutoNumberInsert());
79+
80+
for (int i = 0; i < 5; i++) {
81+
System.gc();
82+
try {
83+
Thread.sleep(50);
84+
} catch (InterruptedException ex) {
85+
Thread.currentThread().interrupt();
86+
}
87+
}
88+
assertTrue(st.getConnection().getDbIO().getTable("t_counter").isAllowAutoNumberInsert(),
89+
"allowAutoNumberInsert must survive a GC pass between DDL and DML");
90+
91+
executeStatements(st,
92+
"INSERT INTO t_counter (cntr, chr, descr) VALUES (3, 'C', 'autoincr OFF, insert arbitrary AutoNumber value')");
93+
}
94+
95+
checkQuery("SELECT cntr, chr FROM t_counter ORDER BY cntr", recs(rec(3, "C")));
96+
}
97+
6498
}

0 commit comments

Comments
 (0)