Don't reuse closed or mismatched MariaDB clients
grabMariaDBClient reused global.MARIADB_CLIENT whenever the requested config matched global.CONFIG. It never checked that the client was made from that config or was still open. init() sets global.CONFIG before creating the client, so a re-init after a close, or a config change, got back a closed or wrong client. - Remember the cache key each client was created with, and compare that - Wrapping close() drops the client from the cache and unsets global.MARIADB_CLIENT, so the next grab or ensureInit makes a new one - resolveClient goes through grabMariaDBClient instead of repeating the global.CONFIG comparison - Remove the queue test's workaround for this Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
@@ -0,0 +1,38 @@
|
|||||||
|
import { afterEach, expect, test } from "bun:test";
|
||||||
|
import grabMariaDBClient from "./grab-mariadb-client";
|
||||||
|
import setMariaDBClient from "./set-mariadb-client";
|
||||||
|
|
||||||
|
// Clients connect lazily, so none of these open a connection
|
||||||
|
const config = { db_name: "bmt_client_cache", ssl_reject_unauthorized: false };
|
||||||
|
|
||||||
|
const originalConfig = global.CONFIG;
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
await global.MARIADB_CLIENT?.close();
|
||||||
|
global.CONFIG = originalConfig;
|
||||||
|
});
|
||||||
|
|
||||||
|
test("reuses the client for the same config", () => {
|
||||||
|
expect(grabMariaDBClient({ config })).toBe(grabMariaDBClient({ config: { ...config } }));
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a closed client is dropped from the cache and the global slot", async () => {
|
||||||
|
setMariaDBClient({ config });
|
||||||
|
const first = global.MARIADB_CLIENT!;
|
||||||
|
|
||||||
|
await first.close();
|
||||||
|
expect(global.MARIADB_CLIENT).toBeUndefined();
|
||||||
|
|
||||||
|
const second = grabMariaDBClient({ config });
|
||||||
|
expect(second).toBeDefined();
|
||||||
|
expect(second).not.toBe(first);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("the global client is not reused for a different config", () => {
|
||||||
|
setMariaDBClient({ config });
|
||||||
|
global.CONFIG = { db_name: "bmt_other" };
|
||||||
|
|
||||||
|
const other = grabMariaDBClient({ config: global.CONFIG });
|
||||||
|
expect(other).not.toBe(global.MARIADB_CLIENT);
|
||||||
|
other?.close();
|
||||||
|
});
|
||||||
@@ -10,6 +10,9 @@ type Params = {
|
|||||||
/** Reuse one client per distinct connection config to avoid exhausting MariaDB connections. */
|
/** Reuse one client per distinct connection config to avoid exhausting MariaDB connections. */
|
||||||
const clientCache = new Map<string, Bun.SQL>();
|
const clientCache = new Map<string, Bun.SQL>();
|
||||||
|
|
||||||
|
/** Cache key each client was created with */
|
||||||
|
const clientKeys = new WeakMap<Bun.SQL, string>();
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Cache key covering everything that affects the connection, not just the
|
* Cache key covering everything that affects the connection, not just the
|
||||||
* database name.
|
* database name.
|
||||||
@@ -36,8 +39,7 @@ export default function grabMariaDBClient({
|
|||||||
// Prefer the process-wide client when it was created from the same config
|
// Prefer the process-wide client when it was created from the same config
|
||||||
if (
|
if (
|
||||||
global.MARIADB_CLIENT &&
|
global.MARIADB_CLIENT &&
|
||||||
global.CONFIG &&
|
clientKeys.get(global.MARIADB_CLIENT) === cacheKey
|
||||||
mariadbClientCacheKey(global.CONFIG) === cacheKey
|
|
||||||
) {
|
) {
|
||||||
clientCache.set(cacheKey, global.MARIADB_CLIENT);
|
clientCache.set(cacheKey, global.MARIADB_CLIENT);
|
||||||
return global.MARIADB_CLIENT;
|
return global.MARIADB_CLIENT;
|
||||||
@@ -71,6 +73,22 @@ export default function grabMariaDBClient({
|
|||||||
adapter: "mariadb",
|
adapter: "mariadb",
|
||||||
} as Bun.SQL.Options);
|
} as Bun.SQL.Options);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A closed client can't run queries, so forget it on close: the next
|
||||||
|
* grab (or `ensureInit`) creates a fresh one.
|
||||||
|
*/
|
||||||
|
const close = MariaDBClient.close.bind(MariaDBClient);
|
||||||
|
MariaDBClient.close = (...args: Parameters<Bun.SQL["close"]>) => {
|
||||||
|
if (clientCache.get(cacheKey) === MariaDBClient) {
|
||||||
|
clientCache.delete(cacheKey);
|
||||||
|
}
|
||||||
|
if (global.MARIADB_CLIENT === MariaDBClient) {
|
||||||
|
global.MARIADB_CLIENT = undefined;
|
||||||
|
}
|
||||||
|
return close(...args);
|
||||||
|
};
|
||||||
|
|
||||||
|
clientKeys.set(MariaDBClient, cacheKey);
|
||||||
clientCache.set(cacheKey, MariaDBClient);
|
clientCache.set(cacheKey, MariaDBClient);
|
||||||
return MariaDBClient;
|
return MariaDBClient;
|
||||||
} catch (error: any) {
|
} catch (error: any) {
|
||||||
|
|||||||
@@ -19,7 +19,8 @@ import {
|
|||||||
declare global {
|
declare global {
|
||||||
var CONFIG: BunMariaDBConfig;
|
var CONFIG: BunMariaDBConfig;
|
||||||
var DB_SCHEMA: BUN_MARIADB_DatabaseSchemaType;
|
var DB_SCHEMA: BUN_MARIADB_DatabaseSchemaType;
|
||||||
var MARIADB_CLIENT: Bun.SQL;
|
/** Unset after the client is closed */
|
||||||
|
var MARIADB_CLIENT: Bun.SQL | undefined;
|
||||||
var CONFIG_FILE_PATH: string;
|
var CONFIG_FILE_PATH: string;
|
||||||
var SCHEMA_FILE_PATH: string;
|
var SCHEMA_FILE_PATH: string;
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-13
@@ -1,6 +1,4 @@
|
|||||||
import grabMariaDBClient, {
|
import grabMariaDBClient from "../functions/grab-mariadb-client";
|
||||||
mariadbClientCacheKey,
|
|
||||||
} from "../functions/grab-mariadb-client";
|
|
||||||
import type {
|
import type {
|
||||||
BunMariaDBConfig,
|
BunMariaDBConfig,
|
||||||
DBInsertReturn,
|
DBInsertReturn,
|
||||||
@@ -29,16 +27,7 @@ export function resolveClient(config?: BunMariaDBConfig): Bun.SQL | undefined {
|
|||||||
return global.MARIADB_CLIENT;
|
return global.MARIADB_CLIENT;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Same connection config as the global client → reuse it
|
// Returns the global client when it was created from the same config
|
||||||
if (
|
|
||||||
global.MARIADB_CLIENT &&
|
|
||||||
(!global.CONFIG ||
|
|
||||||
mariadbClientCacheKey(config) ===
|
|
||||||
mariadbClientCacheKey(global.CONFIG))
|
|
||||||
) {
|
|
||||||
return global.MARIADB_CLIENT;
|
|
||||||
}
|
|
||||||
|
|
||||||
return grabMariaDBClient({ config });
|
return grabMariaDBClient({ config });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+3
-13
@@ -52,20 +52,10 @@ describe.skipIf(!TEST_DB)("Queue tables", () => {
|
|||||||
const createDBSchema = (await import("../src/lib/schema/create-db-schema")).default;
|
const createDBSchema = (await import("../src/lib/schema/create-db-schema")).default;
|
||||||
const appendDefaults = (await import("../src/utils/append-default-fields-to-db-schema")).default;
|
const appendDefaults = (await import("../src/utils/append-default-fields-to-db-schema")).default;
|
||||||
|
|
||||||
/**
|
// The test container uses a self-signed certificate
|
||||||
* The test container uses a self-signed certificate. `db_config` gives
|
global.CONFIG = { db_name: TEST_DB!, ssl_reject_unauthorized: false };
|
||||||
* this file its own client, since other test files close theirs. The
|
|
||||||
* client is created before `global.CONFIG` changes, so the previous
|
|
||||||
* file's closed global client isn't reused.
|
|
||||||
*/
|
|
||||||
const config = {
|
|
||||||
db_name: TEST_DB!,
|
|
||||||
ssl_reject_unauthorized: false,
|
|
||||||
db_config: { max: 10 },
|
|
||||||
};
|
|
||||||
setMariaDBClient({ config });
|
|
||||||
global.CONFIG = config;
|
|
||||||
global.DB_SCHEMA = schema;
|
global.DB_SCHEMA = schema;
|
||||||
|
setMariaDBClient({ config: global.CONFIG });
|
||||||
|
|
||||||
DB = (await import("../src/index")).default;
|
DB = (await import("../src/index")).default;
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user