Skip to content

Commit 3efb82e

Browse files
mlinmlin
committed
improve robustness of extension init routines
1 parent 5789a9b commit 3efb82e

File tree

6 files changed

+52
-40
lines changed

6 files changed

+52
-40
lines changed

‎CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ endif()
1818
FetchContent_Declare(
1919
sqlite_zstd_vfs
2020
GIT_REPOSITORY https://github.com/mlin/sqlite_zstd_vfs.git
21-
GIT_TAG eadb758
21+
GIT_TAG df70c88
2222
)
2323
FetchContent_MakeAvailable(sqlite_zstd_vfs)
2424
FetchContent_MakeAvailable(sqlitecpp)

‎include/genomicsqlite.h‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,11 @@ char *genomicsqlite_default_config_json();
3838
*/
3939
int genomicsqlite_init(int (*)(const char *, sqlite3 **, int, const char *),
4040
int (*)(sqlite3 *, int),
41-
int (*)(sqlite3 *, const char *, const char *, char **), char **pzErrMsg);
41+
int (*)(sqlite3 *, const char *, const char *, char **), int (*)(sqlite3 *),
42+
void (*)(void *), char **pzErrMsg);
4243
#define GENOMICSQLITE_C_INIT(pzErrMsg) \
4344
genomicsqlite_init(sqlite3_open_v2, sqlite3_enable_load_extension, sqlite3_load_extension, \
44-
pzErrMsg);
45+
sqlite3_close, sqlite3_free, pzErrMsg);
4546

4647
/*
4748
* Wrap sqlite3_open() and initialize the "connection" for use with GenomicSQLite. config_json if
@@ -160,11 +161,13 @@ std::string GenomicSQLiteDefaultConfigJSON();
160161
* ...
161162
* }
162163
*/
163-
void GenomicSQLiteInit(int (*open_v2)(const char *, sqlite3 **, int, const char *),
164-
int (*enable_load_extension)(sqlite3 *, int),
165-
int (*load_extension)(sqlite3 *, const char *, const char *, char **));
164+
void GenomicSQLiteInit(int (*)(const char *, sqlite3 **, int, const char *),
165+
int (*)(sqlite3 *, int),
166+
int (*)(sqlite3 *, const char *, const char *, char **), int (*)(sqlite3 *),
167+
void (*)(void *));
166168
#define GENOMICSQLITE_CXX_INIT() \
167-
GenomicSQLiteInit(sqlite3_open_v2, sqlite3_enable_load_extension, sqlite3_load_extension);
169+
GenomicSQLiteInit(sqlite3_open_v2, sqlite3_enable_load_extension, sqlite3_load_extension, \
170+
sqlite3_close, sqlite3_free);
168171

169172
int GenomicSQLiteOpen(const std::string &dbfile, sqlite3 **ppDb, std::string &errmsg_out,
170173
int flags = 0, const std::string &config_json = "{}") noexcept;

‎loaders/sam_into_sqlite.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,11 @@ int main(int argc, char *argv[]) {
275275

276276
try {
277277
// open output database
278+
if (sqlite3_config(SQLITE_CONFIG_MEMSTATUS, 0) != SQLITE_OK ||
279+
sqlite3_config(SQLITE_CONFIG_LOOKASIDE, 4096, 256) != SQLITE_OK) {
280+
throw runtime_error("sqlite3_config() failed");
281+
}
278282
GENOMICSQLITE_CXX_INIT();
279-
sqlite3_config(SQLITE_CONFIG_MEMSTATUS, 0);
280-
sqlite3_config(SQLITE_CONFIG_LOOKASIDE, 2048, 128);
281283
string config_json = R"({"unsafe_load": true, "zstd_level":)" + to_string(level) +
282284
R"(,"inner_page_KiB":)" + to_string(inner_page_KiB) +
283285
R"(,"outer_page_KiB":)" + to_string(outer_page_KiB) + "}";

‎loaders/vcf_into_sqlite.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -667,9 +667,11 @@ int main(int argc, char *argv[]) {
667667
}
668668

669669
// open output database
670+
if (sqlite3_config(SQLITE_CONFIG_MEMSTATUS, 0) != SQLITE_OK ||
671+
sqlite3_config(SQLITE_CONFIG_LOOKASIDE, 4096, 256) != SQLITE_OK) {
672+
throw runtime_error("sqlite3_config() failed");
673+
}
670674
GENOMICSQLITE_CXX_INIT();
671-
sqlite3_config(SQLITE_CONFIG_MEMSTATUS, 0);
672-
sqlite3_config(SQLITE_CONFIG_LOOKASIDE, 2048, 128);
673675
string config_json = R"({"unsafe_load": true, "zstd_level":)" + to_string(level) +
674676
R"(,"inner_page_KiB":)" + to_string(inner_page_KiB) +
675677
R"(,"outer_page_KiB":)" + to_string(outer_page_KiB) + "}";

‎loaders/vcf_lines_into_sqlite.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,11 @@ int main(int argc, char *argv[]) {
136136
}
137137

138138
// open output database
139+
if (sqlite3_config(SQLITE_CONFIG_MEMSTATUS, 0) != SQLITE_OK ||
140+
sqlite3_config(SQLITE_CONFIG_LOOKASIDE, 4096, 256) != SQLITE_OK) {
141+
throw runtime_error("sqlite3_config() failed");
142+
}
139143
GENOMICSQLITE_CXX_INIT();
140-
sqlite3_config(SQLITE_CONFIG_MEMSTATUS, 0);
141-
sqlite3_config(SQLITE_CONFIG_LOOKASIDE, 2048, 128);
142144
auto db = GenomicSQLiteOpen(
143145
outfilename, SQLITE_OPEN_CREATE | SQLITE_OPEN_READWRITE | SQLITE_OPEN_NOMUTEX,
144146
R"( {"unsafe_load": true, "zstd_level": )" + to_string(level) + "}");

‎src/genomicsqlite.cc‎

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,10 @@ string GenomicSQLiteURI(const string &dbfile, const string &config_json = "") {
205205
uri << "&immutable=1";
206206
}
207207
if (cfg.GetBool("$.unsafe_load")) {
208-
uri << "&nolock=1&outer_unsafe";
208+
uri << "&nolock=1&outer_unsafe=1";
209209
}
210210
}
211+
_DBG << uri.str() << endl;
211212
return uri.str();
212213
}
213214

@@ -298,6 +299,7 @@ string GenomicSQLiteTuningSQL(const string &config_json, const string &schema =
298299
for (const auto &p : pragmas) {
299300
out << "; PRAGMA " << p.first << "=" << p.second;
300301
}
302+
_DBG << out.str() << endl;
301303
return out.str();
302304
}
303305

@@ -315,14 +317,13 @@ static void sqlfn_genomicsqlite_tuning_sql(sqlite3_context *ctx, int argc, sqlit
315317

316318
void GenomicSQLiteInit(int (*open_v2)(const char *, sqlite3 **, int, const char *),
317319
int (*enable_load_extension)(sqlite3 *, int),
318-
int (*load_extension)(sqlite3 *, const char *, const char *, char **)) {
319-
static bool ext_loaded = false;
320-
if (!ext_loaded) {
321-
320+
int (*load_extension)(sqlite3 *, const char *, const char *, char **),
321+
int (*close_)(sqlite3 *), void (*free_)(void *)) {
322+
if (!sqlite3_api) {
322323
sqlite3 *memdb = nullptr;
323324
int rc = open_v2(":memory:", &memdb, SQLITE_OPEN_CREATE | SQLITE_OPEN_READWRITE, nullptr);
324325
if (rc != SQLITE_OK) {
325-
sqlite3_close(memdb);
326+
close_(memdb);
326327
throw runtime_error("GenomicSQLiteInit() unable to open temporary SQLite connection");
327328
}
328329
enable_load_extension(memdb, 1);
@@ -331,30 +332,28 @@ void GenomicSQLiteInit(int (*open_v2)(const char *, sqlite3 **, int, const char
331332
if (rc != SQLITE_OK) {
332333
string err = "GenomicSQLiteInit() unable to load the extension" +
333334
(zErrMsg ? ": " + string(zErrMsg) : "");
334-
sqlite3_free(zErrMsg);
335-
sqlite3_close(memdb);
335+
free_(zErrMsg);
336+
close_(memdb);
336337
throw runtime_error(err);
337338
}
338-
sqlite3_free(zErrMsg);
339-
rc = sqlite3_close(memdb);
339+
free_(zErrMsg);
340+
rc = close_(memdb);
340341
if (rc != SQLITE_OK) {
341342
throw runtime_error("GenomicSQLiteInit() unable to close temporary SQLite connection");
342343
}
343-
}
344-
if (open_v2 != sqlite3_api->open_v2) {
344+
} else if (open_v2 != sqlite3_api->open_v2) {
345345
throw std::runtime_error(
346-
"GenomicSQLiteInit() saw inconsistent libsqlite3/libgenomicsqlite library linkage in this process");
346+
"Two distinct copies of SQLite in this process attempted to load Genomics Extension");
347347
}
348-
ext_loaded = true;
349348
}
350349

351-
extern "C" int genomicsqlite_init(int (*open_v2)(const char *, sqlite3 **, int, const char *),
352-
int (*enable_load_extension)(sqlite3 *, int),
353-
int (*load_extension)(sqlite3 *, const char *, const char *,
354-
char **),
355-
char **pzErrMsg) {
350+
extern "C" int
351+
genomicsqlite_init(int (*open_v2)(const char *, sqlite3 **, int, const char *),
352+
int (*enable_load_extension)(sqlite3 *, int),
353+
int (*load_extension)(sqlite3 *, const char *, const char *, char **),
354+
int (*close_)(sqlite3 *), void (*free_)(void *), char **pzErrMsg) {
356355
try {
357-
GenomicSQLiteInit(open_v2, enable_load_extension, load_extension);
356+
GenomicSQLiteInit(open_v2, enable_load_extension, load_extension, close_, free_);
358357
return SQLITE_OK;
359358
} catch (std::bad_alloc &) {
360359
return SQLITE_NOMEM;
@@ -458,7 +457,7 @@ static void sqlfn_genomicsqlite_attach_sql(sqlite3_context *ctx, int argc, sqlit
458457
}
459458

460459
string GenomicSQLiteVacuumIntoSQL(const string &destfile, const string &config_json) {
461-
string desturi = GenomicSQLiteURI(destfile, config_json) + "&outer_unsafe=true";
460+
string desturi = GenomicSQLiteURI(destfile, config_json) + "&outer_unsafe=1";
462461

463462
ConfigParser cfg(config_json);
464463
ostringstream ans;
@@ -1786,6 +1785,14 @@ static int register_genomicsqlite_functions(sqlite3 *db, const char **pzErrMsg,
17861785
*/
17871786
extern "C" int sqlite3_genomicsqlite_init(sqlite3 *db, char **pzErrMsg,
17881787
const sqlite3_api_routines *pApi) {
1788+
if (sqlite3_api && sqlite3_api != pApi) {
1789+
if (pzErrMsg) {
1790+
*pzErrMsg = sqlite3_mprintf(
1791+
"Two distinct copies of SQLite in this process attempted to load Genomics Extension %s",
1792+
GIT_REVISION);
1793+
}
1794+
return SQLITE_ERROR;
1795+
}
17891796
SQLITE_EXTENSION_INIT2(pApi);
17901797

17911798
// The newest SQLite feature currently required is "Generated Columns"
@@ -1800,10 +1807,7 @@ extern "C" int sqlite3_genomicsqlite_init(sqlite3 *db, char **pzErrMsg,
18001807
return SQLITE_ERROR;
18011808
}
18021809

1803-
/* disabled b/c JDBC bindings "legitimately" entail two different dynamically-linked sqlite libs!
1804-
// Check that SQLiteCpp is using the same SQLite library that's loading us. This may not be the
1805-
// case when different versions of SQLite are linked into the running process, one static and
1806-
// one dynamic.
1810+
// Check that SQLiteCpp is using the same SQLite library that's loading us.
18071811
string static_version(pApi->libversion()), dynamic_version("UNKNOWN");
18081812
{
18091813
SQLite::Database tmpdb(":memory:", SQLITE_OPEN_CREATE | SQLITE_OPEN_READWRITE);
@@ -1815,12 +1819,11 @@ extern "C" int sqlite3_genomicsqlite_init(sqlite3 *db, char **pzErrMsg,
18151819
if (static_version != dynamic_version) {
18161820
if (pzErrMsg) {
18171821
*pzErrMsg = sqlite3_mprintf(
1818-
"Two distinct versions of SQLite (%s & %s) detected by Genomics Extension %s. Eliminate static linking of SQLite from the main executable.",
1822+
"Two distinct versions of SQLite (%s & %s) in this process detected by Genomics Extension %s",
18191823
static_version.c_str(), dynamic_version.c_str(), GIT_REVISION);
18201824
}
18211825
return SQLITE_ERROR;
18221826
}
1823-
*/
18241827

18251828
int rc = (new WebVFS::VFS())->Register("web");
18261829
if (rc != SQLITE_OK) {

0 commit comments

Comments
 (0)