Skip to content
This repository was archived by the owner on May 24, 2026. It is now read-only.

Commit abb58bc

Browse files
caesayclaude
andcommitted
Fix thread-safety race in all MSI functions
Move handle_table_get_typed calls inside libmsi_global_lock region for all functions. Previously, the handle lookup happened outside the lock, creating a race where another thread could close the underlying database between lookup and use, invalidating internal file handles and table pointers. The pattern change for every function: Before: lookup → lock → use → unlock After: lock → lookup → use → unlock This eliminates the window where concurrent MsiCloseHandle on a database could invalidate the internal state of objects being used by other threads. The protective g_object_ref/unref pairs are no longer needed since the lock now covers the entire operation. Fixes AccessViolationException when running WiX tests in parallel. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2a6210b commit abb58bc

4 files changed

Lines changed: 198 additions & 175 deletions

File tree

msi-interop/database.c

Lines changed: 50 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,13 @@ MsiOpenDatabaseW(LPCWSTR szDatabasePath, LPCWSTR szPersist, MSIHANDLE *phDatabas
183183
UINT WINAPI
184184
MsiDatabaseCommit(MSIHANDLE hDatabase)
185185
{
186+
libmsi_global_lock();
187+
186188
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
187-
if (!db)
189+
if (!db) {
190+
libmsi_global_unlock();
188191
return ERROR_INVALID_HANDLE;
189-
190-
libmsi_global_lock();
192+
}
191193

192194
GError *error = NULL;
193195
gboolean ok = libmsi_database_commit(db, &error);
@@ -208,11 +210,14 @@ MsiDatabaseCommit(MSIHANDLE hDatabase)
208210
MSIDBSTATE WINAPI
209211
MsiGetDatabaseState(MSIHANDLE hDatabase)
210212
{
213+
libmsi_global_lock();
214+
211215
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
212-
if (!db)
216+
if (!db) {
217+
libmsi_global_unlock();
213218
return MSIDBSTATE_ERROR;
219+
}
214220

215-
libmsi_global_lock();
216221
MSIDBSTATE ret = libmsi_database_is_readonly(db) ? MSIDBSTATE_READ : MSIDBSTATE_WRITE;
217222
g_object_unref(db);
218223
libmsi_global_unlock();
@@ -230,22 +235,17 @@ MsiDatabaseGetPrimaryKeysA(MSIHANDLE hDatabase, LPCSTR szTableName, MSIHANDLE *p
230235
if (!szTableName || !phRecord)
231236
return ERROR_INVALID_PARAMETER;
232237

233-
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
234-
if (!db)
235-
return ERROR_INVALID_HANDLE;
236-
237-
/*
238-
* Use internal API to avoid LIBMSI_IS_DATABASE type-check macro.
239-
* Add protective ref/unref to match the public API's bracketing.
240-
*/
241238
libmsi_global_lock();
242239

243-
g_object_ref(db); /* protective ref (matches public API) */
240+
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
241+
if (!db) {
242+
libmsi_global_unlock();
243+
return ERROR_INVALID_HANDLE;
244+
}
244245

245246
LibmsiRecord *rec = NULL;
246247
unsigned r = _libmsi_database_get_primary_keys(db, szTableName, &rec);
247248

248-
g_object_unref(db); /* drop protective ref */
249249
g_object_unref(db); /* drop handle_table_get_typed ref */
250250

251251
if (r != LIBMSI_RESULT_SUCCESS || !rec) {
@@ -290,21 +290,16 @@ MsiDatabaseIsTablePersistentA(MSIHANDLE hDatabase, LPCSTR szTableName)
290290
if (!szTableName)
291291
return MSICONDITION_ERROR;
292292

293-
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
294-
if (!db)
295-
return MSICONDITION_ERROR;
296-
297-
/*
298-
* Use internal API to avoid LIBMSI_IS_DATABASE type-check macro.
299-
* Add protective ref/unref to match the public API's bracketing.
300-
*/
301293
libmsi_global_lock();
302294

303-
g_object_ref(db); /* protective ref (matches public API) */
295+
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
296+
if (!db) {
297+
libmsi_global_unlock();
298+
return MSICONDITION_ERROR;
299+
}
304300

305301
LibmsiCondition r = _libmsi_database_is_table_persistent(db, szTableName);
306302

307-
g_object_unref(db); /* drop protective ref */
308303
g_object_unref(db); /* drop handle_table_get_typed ref */
309304

310305
libmsi_global_unlock();
@@ -340,15 +335,18 @@ MsiDatabaseImportA(MSIHANDLE hDatabase, LPCSTR szFolderPath, LPCSTR szFileName)
340335
if (!szFolderPath || !szFileName)
341336
return ERROR_INVALID_PARAMETER;
342337

343-
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
344-
if (!db)
345-
return ERROR_INVALID_HANDLE;
346-
347338
/* Build full path: folder + separator + file */
348339
char *full_path = g_build_filename(szFolderPath, szFileName, NULL);
349340

350341
libmsi_global_lock();
351342

343+
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
344+
if (!db) {
345+
libmsi_global_unlock();
346+
g_free(full_path);
347+
return ERROR_INVALID_HANDLE;
348+
}
349+
352350
GError *error = NULL;
353351
gboolean ok = libmsi_database_import(db, full_path, &error);
354352
g_free(full_path);
@@ -394,10 +392,6 @@ MsiDatabaseExportA(MSIHANDLE hDatabase, LPCSTR szTableName, LPCSTR szFolderPath,
394392
if (!szTableName || !szFolderPath || !szFileName)
395393
return ERROR_INVALID_PARAMETER;
396394

397-
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
398-
if (!db)
399-
return ERROR_INVALID_HANDLE;
400-
401395
/* Build the full path and open it as a file descriptor */
402396
char *full_path = g_build_filename(szFolderPath, szFileName, NULL);
403397

@@ -408,13 +402,22 @@ MsiDatabaseExportA(MSIHANDLE hDatabase, LPCSTR szTableName, LPCSTR szFolderPath,
408402
#endif
409403
g_free(full_path);
410404

411-
if (fd < 0) {
412-
g_object_unref(db);
405+
if (fd < 0)
413406
return ERROR_FUNCTION_FAILED;
414-
}
415407

416408
libmsi_global_lock();
417409

410+
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
411+
if (!db) {
412+
#ifdef _WIN32
413+
_close(fd);
414+
#else
415+
close(fd);
416+
#endif
417+
libmsi_global_unlock();
418+
return ERROR_INVALID_HANDLE;
419+
}
420+
418421
GError *error = NULL;
419422
gboolean ok = libmsi_database_export(db, szTableName, fd, &error);
420423

@@ -466,18 +469,21 @@ MsiDatabaseExportW(MSIHANDLE hDatabase, LPCWSTR szTableName, LPCWSTR szFolderPat
466469
UINT WINAPI
467470
MsiDatabaseMergeA(MSIHANDLE hDatabase, MSIHANDLE hDatabaseMerge, LPCSTR szTableName)
468471
{
472+
libmsi_global_lock();
473+
469474
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
470-
if (!db)
475+
if (!db) {
476+
libmsi_global_unlock();
471477
return ERROR_INVALID_HANDLE;
478+
}
472479

473480
LibmsiDatabase *db_merge = (LibmsiDatabase *)handle_table_get_typed(hDatabaseMerge, HANDLE_DATABASE);
474481
if (!db_merge) {
475482
g_object_unref(db);
483+
libmsi_global_unlock();
476484
return ERROR_INVALID_HANDLE;
477485
}
478486

479-
libmsi_global_lock();
480-
481487
GError *error = NULL;
482488
gboolean ok = libmsi_database_merge(db, db_merge, szTableName, &error);
483489
g_object_unref(db);
@@ -518,21 +524,16 @@ MsiDatabaseApplyTransformA(MSIHANDLE hDatabase, LPCSTR szTransformFile,
518524
if (!szTransformFile)
519525
return ERROR_INVALID_PARAMETER;
520526

521-
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
522-
if (!db)
523-
return ERROR_INVALID_HANDLE;
524-
525-
/*
526-
* Use internal API to avoid LIBMSI_IS_DATABASE type-check macro.
527-
* Add protective ref/unref to match the public API's bracketing.
528-
*/
529527
libmsi_global_lock();
530528

531-
g_object_ref(db); /* protective ref (matches public API) */
529+
LibmsiDatabase *db = (LibmsiDatabase *)handle_table_get_typed(hDatabase, HANDLE_DATABASE);
530+
if (!db) {
531+
libmsi_global_unlock();
532+
return ERROR_INVALID_HANDLE;
533+
}
532534

533535
unsigned r = _libmsi_database_apply_transform(db, szTransformFile);
534536

535-
g_object_unref(db); /* drop protective ref */
536537
g_object_unref(db); /* drop handle_table_get_typed ref */
537538

538539
libmsi_global_unlock();

0 commit comments

Comments
 (0)