Skip to content

Commit 1216a58

Browse files
rogerbinnsrogerbinns
committed
WIP getting rid of DBMUTEX_FORCE
It didn't work under some conditions and wasn't the right way Py_AddPendingCall is appropriate Its error return needs to be handled Connection_init still has an error path that needs fixing
1 parent 00a153d commit 1216a58

7 files changed

Lines changed: 134 additions & 48 deletions

File tree

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ compile-win: ## Builds and tests against all the Python versions on Windows
232232
# other packages were skipped due to metadata issues
233233
compile-win-one: ## Does one Windows build - set PYTHON variable
234234
$(PYTHON) -m pip install --upgrade --upgrade-strategy eager pip wheel setuptools trio anyio
235-
$(PYTHON) -m pip uninstall -y apsw trio
235+
$(PYTHON) -m pip uninstall -y apsw
236236
copy tools\\setup-pypi.cfg setup.apsw
237237
$(PYTHON) -m pip --no-cache-dir wheel -v .
238238
cmd /c FOR %i in (*.whl) DO $(PYTHON) -m pip --no-cache-dir install --force-reinstall %i

‎src/backup.c‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,22 +121,29 @@ APSWBackup_close_internal(APSWBackup *self, int force)
121121
return setexc;
122122
}
123123

124-
static void
125-
APSWBackup_dealloc(PyObject *self_)
124+
static int
125+
APSWBackup_dealloc_mutex(void *self_)
126126
{
127127
APSWBackup *self = (APSWBackup *)self_;
128-
APSW_CLEAR_WEAKREFS;
129-
130128
if (self->backup)
131129
{
132-
DBMUTEX_FORCE(self->source->dbmutex);
133-
DBMUTEX_FORCE(self->dest->dbmutex);
134-
130+
DBMUTEX_RETRY_2(self->source, self->dest, APSWBackup_dealloc_mutex);
135131
APSWBackup_close_internal(self, 2);
136132
}
137133
Py_CLEAR(self->done);
138134

139135
Py_TpFree(self_);
136+
137+
return 0;
138+
}
139+
140+
static void
141+
APSWBackup_dealloc(PyObject *self_)
142+
{
143+
APSWBackup *self = (APSWBackup *)self_;
144+
APSW_CLEAR_WEAKREFS;
145+
146+
APSWBackup_dealloc_mutex(self);
140147
}
141148

142149
/** .. method:: step(npages: int = -1) -> bool

‎src/blob.c‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,17 +195,25 @@ APSWBlob_close_internal(APSWBlob *self, int force)
195195
return setexc;
196196
}
197197

198-
static void
199-
APSWBlob_dealloc(PyObject *self_)
198+
static int
199+
APSWBlob_dealloc_mutex(void *self_)
200200
{
201201
APSWBlob *self = (APSWBlob *)self_;
202-
APSW_CLEAR_WEAKREFS;
202+
DBMUTEX_RETRY(self->connection, APSWBlob_dealloc_mutex);
203203

204-
if (self->connection)
205-
DBMUTEX_FORCE(self->connection->dbmutex);
206204
APSWBlob_close_internal(self, 2);
207205

208206
Py_TpFree(self_);
207+
return 0;
208+
}
209+
210+
static void
211+
APSWBlob_dealloc(PyObject *self_)
212+
{
213+
APSWBlob *self = (APSWBlob *)self_;
214+
APSW_CLEAR_WEAKREFS;
215+
216+
APSWBlob_dealloc_mutex(self);
209217
}
210218

211219
/* If the blob is closed, we return the same error as normal python files */

‎src/connection.c‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -548,14 +548,12 @@ Connection_aclose(PyObject *self_, PyObject *const *fast_args, Py_ssize_t fast_n
548548
return async_return_value(Py_None);
549549
}
550550

551-
static void
552-
Connection_dealloc(PyObject *self_)
551+
static int
552+
Connection_dealloc_mutex(void *self_)
553553
{
554554
Connection *self = (Connection *)self_;
555-
PyObject_GC_UnTrack(self_);
556-
APSW_CLEAR_WEAKREFS;
555+
DBMUTEX_RETRY(self, Connection_dealloc_mutex);
557556

558-
DBMUTEX_FORCE(self->dbmutex);
559557
Connection_close_internal(self, 2);
560558

561559
/* Our dependents all hold a refcount on us, so they must have all
@@ -564,6 +562,18 @@ Connection_dealloc(PyObject *self_)
564562
Py_CLEAR(self->dependents);
565563

566564
Py_TpFree(self_);
565+
566+
return 0;
567+
}
568+
569+
static void
570+
Connection_dealloc(PyObject *self_)
571+
{
572+
Connection *self = (Connection *)self_;
573+
PyObject_GC_UnTrack(self_);
574+
APSW_CLEAR_WEAKREFS;
575+
576+
Connection_dealloc_mutex(self);
567577
}
568578

569579
/** .. method:: __init__(filename: str, flags: int = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, vfs: str | None = None, statementcachesize: int = 100)

‎src/cursor.c‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -282,28 +282,30 @@ APSWCursor_close_internal(APSWCursor *self, int force)
282282
return 0;
283283
}
284284

285+
static int
286+
APSWCursor_dealloc_mutex(void * self_)
287+
{
288+
APSWCursor *self = (APSWCursor *)self_;
289+
DBMUTEX_RETRY(self->connection, APSWCursor_dealloc_mutex);
290+
291+
APSWCursor_close_internal(self, 2);
292+
293+
Py_TpFree(self_);
294+
295+
return 0;
296+
}
297+
285298
static void
286299
APSWCursor_dealloc(PyObject *self_)
287300
{
288301
APSWCursor *self = (APSWCursor *)self_;
289-
/* dealloc is not allowed to return an exception or
290-
clear the current exception */
291-
PY_ERR_FETCH(exc_save);
292302

293303
assert(!self->in_query);
294304

295305
PyObject_GC_UnTrack(self);
296306
APSW_CLEAR_WEAKREFS;
297307

298-
if (self->connection)
299-
DBMUTEX_FORCE(self->connection->dbmutex);
300-
APSWCursor_close_internal(self, 2);
301-
302-
if (PyErr_Occurred())
303-
apsw_write_unraisable(NULL);
304-
305-
PY_ERR_RESTORE(exc_save);
306-
Py_TpFree(self_);
308+
APSWCursor_dealloc_mutex(self);
307309
}
308310

309311
/** .. method:: __init__(connection: Connection)

‎src/session.c‎

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -365,9 +365,12 @@ APSWSession_init(PyObject *self_, PyObject *args, PyObject *kwargs)
365365
static void
366366
APSWSession_close_internal(APSWSession *self)
367367
{
368+
sqlite3_mutex *mutex = NULL;
369+
368370
if (self->session)
369371
{
370-
assert(sqlite3_mutex_held(self->connection->dbmutex));
372+
mutex = self->connection->dbmutex;
373+
assert(sqlite3_mutex_held(mutex));
371374
sqlite3session_delete(self->session);
372375
self->session = NULL;
373376
}
@@ -376,21 +379,30 @@ APSWSession_close_internal(APSWSession *self)
376379

377380
if (self->connection)
378381
{
379-
sqlite3_mutex_leave(self->connection->dbmutex);
382+
sqlite3_mutex_leave(mutex);
380383
Connection_remove_dependent(self->connection, (PyObject *)self);
381384
}
382385
Py_CLEAR(self->connection);
383386
}
384387

385-
static void
386-
APSWSession_dealloc(PyObject *self_)
388+
static int
389+
APSWSession_dealloc_mutex(void *self_)
387390
{
388391
APSWSession *self = (APSWSession *)self_;
389-
sqlite3_mutex *mutex = self->connection ? self->connection->dbmutex : NULL;
390-
if (mutex)
391-
DBMUTEX_FORCE(mutex);
392+
393+
DBMUTEX_RETRY(self->connection, APSWSession_dealloc_mutex);
392394
APSWSession_close_internal(self);
393395
Py_TpFree(self_);
396+
397+
return 0;
398+
}
399+
400+
static void
401+
APSWSession_dealloc(PyObject *self_)
402+
{
403+
APSWSession *self = (APSWSession *)self_;
404+
405+
APSWSession_dealloc_mutex(self);
394406
}
395407

396408
/** .. method:: close() -> None
@@ -2048,26 +2060,30 @@ APSWChangesetBuilder_close_internal(APSWChangesetBuilder *self)
20482060
}
20492061
if (self->connection)
20502062
{
2051-
assert(sqlite3_mutex_held(self->connection->dbmutex));
20522063
Connection_remove_dependent(self->connection, (PyObject *)self);
20532064
Py_CLEAR(self->connection);
20542065
}
20552066
}
20562067

2057-
static void
2058-
APSWChangesetBuilder_dealloc(PyObject *self_)
2068+
static int
2069+
APSWChangesetBuilder_dealloc_mutex(void *self_)
20592070
{
20602071
APSWChangesetBuilder *self = (APSWChangesetBuilder *)self_;
2061-
sqlite3_mutex *mutex = NULL;
2062-
if (self->connection)
2063-
{
2064-
mutex = self->connection->dbmutex;
2065-
DBMUTEX_FORCE(mutex);
2066-
}
2072+
DBMUTEX_RETRY(self->connection, APSWChangesetBuilder_dealloc_mutex);
2073+
2074+
sqlite3_mutex *mutex = (self->connection) ? self->connection->dbmutex : NULL;
20672075
APSWChangesetBuilder_close_internal(self);
2068-
if (mutex)
2069-
sqlite3_mutex_leave(mutex);
2076+
sqlite3_mutex_leave(mutex);
2077+
20702078
Py_TpFree(self_);
2079+
return 0;
2080+
}
2081+
2082+
static void
2083+
APSWChangesetBuilder_dealloc(PyObject *self_)
2084+
{
2085+
APSWChangesetBuilder *self = (APSWChangesetBuilder *)self_;
2086+
APSWChangesetBuilder_dealloc_mutex(self);
20712087
}
20722088

20732089
/** .. method:: close() -> None

‎src/util.c‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,49 @@
6969
} \
7070
} while (0)
7171

72+
#define DBMUTEX_RETRY_2(conn1, conn2, func) \
73+
do \
74+
{ \
75+
sqlite3_mutex *mutex_one = NULL; \
76+
if (conn1) \
77+
{ \
78+
mutex_one = (conn1)->dbmutex; \
79+
switch (sqlite3_mutex_try(mutex_one)) \
80+
{ \
81+
case SQLITE_MISUSE: \
82+
PyErr_SetString(ExcForkingViolation, \
83+
"SQLite object allocated in one process is being used in another (across a fork)"); \
84+
return -1; \
85+
case SQLITE_BUSY: \
86+
return Py_AddPendingCall(func, self); \
87+
case SQLITE_OK: \
88+
break; \
89+
default: \
90+
Py_UNREACHABLE(); \
91+
} \
92+
} \
93+
if (conn2) \
94+
{ \
95+
switch (sqlite3_mutex_try((conn2)->dbmutex)) \
96+
{ \
97+
case SQLITE_MISUSE: \
98+
PyErr_SetString(ExcForkingViolation, \
99+
"SQLite object allocated in one process is being used in another (across a fork)"); \
100+
sqlite3_mutex_leave(mutex_one); \
101+
return -1; \
102+
case SQLITE_BUSY: \
103+
sqlite3_mutex_leave(mutex_one); \
104+
return Py_AddPendingCall(func, self); \
105+
case SQLITE_OK: \
106+
break; \
107+
default: \
108+
Py_UNREACHABLE(); \
109+
} \
110+
} \
111+
} while (0)
112+
113+
#define DBMUTEX_RETRY(connection, func) DBMUTEX_RETRY_2(connection, (Connection*)0, func)
114+
72115
/*
73116
The default Python PyErr_WriteUnraisable is almost useless, and barely used
74117
by CPython. It gives the developer no clue whatsoever where in

0 commit comments

Comments
 (0)