Skip to content

Commit

Permalink
Grab most of the changes suggested in #363.
Browse files Browse the repository at this point in the history
Capture the PID at environment creation time and don't delete
transactions when we're not on that PID.  This will catch some common
accidental misuses.
  • Loading branch information
jnwatson committed Jan 9, 2025
1 parent 07a760e commit ee93a94
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 16 deletions.
28 changes: 18 additions & 10 deletions lmdb/cpython.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2024 The py-lmdb authors, all rights reserved.
* Copyright 2013-2025 The py-lmdb authors, all rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted only as authorized by the OpenLDAP
Expand Down Expand Up @@ -166,11 +166,13 @@ struct EnvObject {
DbObject *main_db;
/** 1 if env opened read-only; transactions must always be read-only. */
int readonly;
/** Spare read-only transaction . */
/** Spare read-only transaction. */
struct MDB_txn *spare_txn;
/** Maximum number of spare transactions. In cpython only 0 and 1 are supported.
* If process will be forked, this must be set to 0. */
int max_spare_txns;
/** Process ID of the process this Environment was opened in. */
pid_t pid;
};

/** TransObject.flags bitfield values. */
Expand Down Expand Up @@ -1212,6 +1214,7 @@ env_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
self->env = NULL;
self->spare_txn = NULL;
self->max_spare_txns = arg.max_spare_txns;
self->pid = getpid();

if((rc = mdb_env_create(&self->env))) {
err_set("mdb_env_create", rc);
Expand Down Expand Up @@ -3217,16 +3220,21 @@ trans_dealloc(TransObject *self)
PyObject_ClearWeakRefs((PyObject *) self);
}

if(txn && self->env && !self->env->spare_txn &&
self->env->max_spare_txns && (self->flags & TRANS_RDONLY)) {
MDEBUG("caching trans")
mdb_txn_reset(txn);
self->env->spare_txn = txn;
self->txn = NULL;
if(self->env && self->env->pid == getpid()) {
if(txn && self->env && !self->env->spare_txn &&
self->env->max_spare_txns && (self->flags & TRANS_RDONLY)) {
MDEBUG("caching trans")
mdb_txn_reset(txn);
self->env->spare_txn = txn;
self->txn = NULL;
}
MDEBUG("deleting trans")
trans_clear(self);
}
else {
MDEBUG("In forked process, not deleting trans");
}

MDEBUG("deleting trans")
trans_clear(self);
PyObject_Del(self);
}

Expand Down
1 change: 0 additions & 1 deletion tests/env_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,6 @@ class SpareTxnTest(unittest.TestCase):
def tearDown(self):
testlib.cleanup()

@unittest.skip('Temporarily removed this functionality')
def test_none(self):
_, env = testlib.temp_env(max_spare_txns=0)
assert 0 == reader_count(env)
Expand Down
36 changes: 31 additions & 5 deletions tests/txn_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
ULONG_0002 = struct.pack('L', 2) # L != size_t


class InitTest(unittest.TestCase):
class InitTest(testlib.LmdbTest):
def tearDown(self):
testlib.cleanup()

Expand Down Expand Up @@ -141,14 +141,40 @@ def test_buffers(self):
txn.abort()

@unittest.skipIf(sys.platform.startswith('win'), "No fork on Windows")
def test_fork(self):
def test_cached_txn_across_fork(self):
_, env = testlib.temp_env()
txn = env.begin(write=False)
del txn

if (os.fork() != 0):
os.wait()
txn = env.begin(write=False)
if os.fork() != 0:
# I am the parent
os.wait() # Wait for child to finish
txn = env.begin(write=False) # Used to raise MDB_BAD_RSLOT (#346)

@unittest.skipIf(sys.platform.startswith('win'), "No fork on Windows")
def test_child_deleting_transaction(self):
_, env = testlib.temp_env()
with env.begin(write=True) as txn:
txn.put(b'a', b'foo')
txn = env.begin()

r = env.readers()
assert r != '(no active readers)\n'

pid = os.fork()
if pid == 0:
del txn # I am the child
os._exit(0)

os.waitpid(pid, 0)
# Make sure my child didn't mess with my environment
r2 = env.readers()
assert r2 == r, '%r != %r' % (r2, r)
assert txn.get(b'a') == b'foo'
print
del txn
r3 = env.readers()
print(r, r2, r3)


class ContextManagerTest(unittest.TestCase):
Expand Down

0 comments on commit ee93a94

Please sign in to comment.