Skip to content

Commit ed27a91

Browse files
committed
data to pyobject mapping doesn't work
there could be multiple owners so we don't know which one was used this time. consequently the data is now always copied when binding. also need to use dealloc slot not the finalize slot
1 parent b550675 commit ed27a91

3 files changed

Lines changed: 22 additions & 80 deletions

File tree

src/apsw.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,9 +1069,6 @@ static PyObject *
10691069
apsw_fini(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(unused))
10701070
{
10711071
fini_apsw_strings();
1072-
#ifdef SQLITE_ENABLE_CARRAY
1073-
free(carray_owner_array);
1074-
#endif
10751072
Py_RETURN_NONE;
10761073
}
10771074
#endif

src/carray.c

Lines changed: 18 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,60 +1,8 @@
11

2-
/* The destructor API does a callback on the void * passed in which means
3-
we can't get back to the PyObject that owns it. This array is
4-
used to store the mapping. Hopefully it can be deleted at some point. */
5-
6-
typedef struct
7-
{
8-
void *aData;
9-
PyObject *owner;
10-
} carray_pyobject_owner;
11-
12-
static carray_pyobject_owner *carray_owner_array = 0;
13-
size_t carray_owner_array_length = 0;
14-
15-
/* returns 0 on success, -1 on failure */
16-
static int
17-
carray_add_owner(void *aData, PyObject *owner)
18-
{
19-
assert(aData && owner);
20-
21-
for (size_t i = 0; i < carray_owner_array_length; i++)
22-
{
23-
if (!carray_owner_array[i].aData)
24-
{
25-
carray_owner_array[i].aData = aData;
26-
carray_owner_array[i].owner = owner;
27-
return 0;
28-
}
29-
}
30-
carray_pyobject_owner *new_array
31-
= realloc(carray_owner_array, sizeof(carray_pyobject_owner) * (carray_owner_array_length + 1));
32-
if (!new_array)
33-
return -1;
34-
carray_owner_array = new_array;
35-
carray_owner_array[carray_owner_array_length].aData = aData;
36-
carray_owner_array[carray_owner_array_length].owner = owner;
37-
carray_owner_array_length++;
38-
return 0;
39-
}
40-
41-
/* returns the owner and clears the entry - ie can only be called once */
42-
static PyObject *
43-
carray_get_owner(void *aData)
44-
{
45-
for (size_t i = 0; i < carray_owner_array_length; i++)
46-
{
47-
if (carray_owner_array[i].aData == aData)
48-
{
49-
PyObject *owner = carray_owner_array[i].owner;
50-
51-
carray_owner_array[i].aData = 0;
52-
carray_owner_array[i].owner = 0;
53-
return owner;
54-
}
55-
}
56-
Py_UNREACHABLE();
57-
}
2+
/* The destructor API does a callback on the void * passed in, but we
3+
could have multiple PyObject owners referencing the same array so the
4+
data always has to be duplicated when passed to sqlite3_carray_bind/.
5+
*/
586

597
typedef struct
608
{
@@ -110,36 +58,35 @@ CArrayBind_init(PyObject *self_, PyObject *args, PyObject *kwargs)
11058
self->aData = self->view.buf;
11159
self->nData = self->view.len / 8;
11260
self->mFlags = flags;
113-
114-
if (carray_add_owner(self->aData, (PyObject *)self))
115-
{
116-
PyErr_NoMemory();
117-
goto error;
118-
}
119-
12061
return 0;
62+
12163
error:
12264
if (res == 0)
12365
PyBuffer_Release(&self->view);
66+
self->aData = 0;
12467
return -1;
12568
}
12669

12770
static void
128-
carray_bind_destructor(void *value)
71+
CArrayBind_dealloc(PyObject *self_)
12972
{
130-
PyGILState_STATE gilstate = PyGILState_Ensure();
131-
CArrayBind *self = (CArrayBind *)carray_get_owner(value);
132-
PyBuffer_Release(&self->view);
133-
/* undo incref in APSWCursor_dobinding */
134-
Py_DECREF((PyObject *)self);
135-
PyGILState_Release(gilstate);
73+
74+
CArrayBind *self = (CArrayBind *)self_;
75+
76+
if (self->aData)
77+
{
78+
PyBuffer_Release(&self->view);
79+
self->aData = 0;
80+
}
81+
Py_TpFree(self_);
13682
}
13783

13884
static PyTypeObject CArrayBindType = {
13985
PyVarObject_HEAD_INIT(NULL, 0).tp_name = "apsw.carray",
14086
.tp_basicsize = sizeof(CArrayBind),
141-
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
87+
.tp_flags = Py_TPFLAGS_DEFAULT,
14288
.tp_init = CArrayBind_init,
14389
.tp_new = PyType_GenericNew,
90+
.tp_dealloc = CArrayBind_dealloc,
14491
.tp_doc = Apsw_carray_DOC,
14592
};

src/cursor.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -699,12 +699,9 @@ APSWCursor_dobinding(APSWCursor *self, int arg, PyObject *obj)
699699
#ifdef SQLITE_ENABLE_CARRAY
700700
else if (PyObject_TypeCheck(obj, &CArrayBindType) == 1)
701701
{
702-
/* This is decref in carray_bind_destructor and ensures the buffer
703-
lives at least as long as SQLite wants it to */
704-
Py_INCREF(obj);
705702
CArrayBind *cab = (CArrayBind *)obj;
706703
res = sqlite3_carray_bind(self->statement->vdbestatement, arg, cab->aData, cab->nData, cab->mFlags,
707-
carray_bind_destructor);
704+
SQLITE_TRANSIENT);
708705
}
709706
#endif
710707
else if (CONVERT_BINDING)
@@ -2206,17 +2203,18 @@ PyObjectBind_init(PyObject *self_, PyObject *args, PyObject *kwargs)
22062203
}
22072204

22082205
static void
2209-
PyObjectBind_finalize(PyObject *self)
2206+
PyObjectBind_dealloc(PyObject *self)
22102207
{
22112208
Py_CLEAR(((PyObjectBind *)self)->object);
2209+
Py_TpFree(self);
22122210
}
22132211

22142212
static PyTypeObject PyObjectBindType = {
22152213
PyVarObject_HEAD_INIT(NULL, 0).tp_name = "apsw.pyobject",
22162214
.tp_basicsize = sizeof(PyObjectBind),
22172215
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
22182216
.tp_init = PyObjectBind_init,
2219-
.tp_finalize = PyObjectBind_finalize,
2217+
.tp_dealloc = PyObjectBind_dealloc,
22202218
.tp_new = PyType_GenericNew,
22212219
.tp_doc = Apsw_pyobject_DOC,
22222220
};

0 commit comments

Comments
 (0)