Skip to content

Commit 615561a

Browse files
nguyenvjohnkerl
andauthored
[python][c++] Correctly manage lifetime of SOMAVFS with SOMAVFSFilebuf (#3976)
* [python] Correctly manage lifetime of `SOMAVFS` with `SOMAVFSFilebuf` * Update apis/python/tests/test_basic_anndata_io.py Co-authored-by: John Kerl <kerl.john.r@gmail.com> * collect vfs --------- Co-authored-by: John Kerl <kerl.john.r@gmail.com>
1 parent a8f5e1a commit 615561a

File tree

2 files changed

+20
-6
lines changed

2 files changed

+20
-6
lines changed

apis/python/src/tiledbsoma/soma_vfs.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@ class SOMAVFS : public tiledb::VFS {
3333
class SOMAVFSFilebuf : public tiledb::impl::VFSFilebuf {
3434
private:
3535
std::streamsize offset_ = 0;
36-
SOMAVFS vfs_;
36+
std::shared_ptr<SOMAVFS> vfs_;
3737
std::ios::openmode openmode_;
3838

3939
public:
40-
SOMAVFSFilebuf(const VFS& vfs)
41-
: tiledb::impl::VFSFilebuf(vfs)
40+
SOMAVFSFilebuf(std::shared_ptr<SOMAVFS> vfs)
41+
: tiledb::impl::VFSFilebuf(*vfs)
4242
, vfs_(vfs){};
4343

4444
SOMAVFSFilebuf* open(const std::string& uri, std::ios::openmode openmode) {
@@ -59,7 +59,7 @@ class SOMAVFSFilebuf : public tiledb::impl::VFSFilebuf {
5959
} else if (whence == 1) {
6060
offset_ += seekoff(offset, std::ios::cur, std::ios::in);
6161
} else if (whence == 2) {
62-
offset_ = vfs_.file_size(get_uri()) -
62+
offset_ = vfs_->file_size(get_uri()) -
6363
seekoff(offset, std::ios::end, std::ios::in);
6464
} else {
6565
TPY_ERROR_LOC(
@@ -124,15 +124,15 @@ class SOMAVFSFilebuf : public tiledb::impl::VFSFilebuf {
124124
};
125125

126126
void load_soma_vfs(py::module& m) {
127-
py::class_<SOMAVFS>(m, "SOMAVFS")
127+
py::class_<SOMAVFS, std::shared_ptr<SOMAVFS>>(m, "SOMAVFS")
128128
.def(
129129
py::init([](std::shared_ptr<SOMAContext> context) {
130130
return SOMAVFS(*context->tiledb_ctx());
131131
}),
132132
"ctx"_a);
133133

134134
py::class_<SOMAVFSFilebuf>(m, "SOMAVFSFilebuf")
135-
.def(py::init<const SOMAVFS&>())
135+
.def(py::init<const std::shared_ptr<SOMAVFS>&>())
136136
.def(
137137
"open",
138138
[](SOMAVFSFilebuf& buf, const std::string& uri) {

apis/python/tests/test_basic_anndata_io.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import gc
34
import json
45
import random
56
import tempfile
@@ -1557,3 +1558,16 @@ def test_from_anndata_byteorder_63459(tmp_path, conftest_pbmc_small):
15571558
with tiledbsoma.Experiment.open(exp_uri) as E:
15581559
new_ad = tiledbsoma.io.to_anndata(E, "RNA", X_layer_name="data")
15591560
assert ad.uns["X"] == new_ad.uns["X"]
1561+
1562+
1563+
def test_vfs_lifetime_65831():
1564+
# https://app.shortcut.com/tiledb-inc/story/65831/python-c-segv-memory-issues-in-somavfs-somavfsfilebuf
1565+
context = tiledbsoma.SOMATileDBContext()
1566+
vfs = tiledbsoma.pytiledbsoma.SOMAVFS(context.native_context)
1567+
fb = tiledbsoma.pytiledbsoma.SOMAVFSFilebuf(vfs).open(
1568+
str(TESTDATA / "pbmc-small.h5ad")
1569+
)
1570+
del vfs
1571+
gc.collect() # Make sure that vfs is freed
1572+
fb.read(100) # Implicitly ensure that read does not segfault
1573+
fb.close()

0 commit comments

Comments
 (0)