Prevent some accidental use of LMDB objects by child process (#375)

Default max_spare_txns on cpython impl to 0.

Also, don't create spare transactions if the above parameter is true.

This prevents hanging open transactions across forks. This was exhibited
by a MDB_BAD_RSLOT error.

Fixes #346.

Grab most of the changes suggested in #363. Thank you to callumwalker for 
their contribution. This records the PID at environment creation time and
prevents deletion of transactions when we're not on that PID.

ChangeLog

@@ -31,7 +31,7 @@
 * Add Python 3.10 support.
 
 * Fix crash relating to caching of transactions.  The 'max_spare_txns'
-  parameter to Environment/open is currently ignored.
+  parameter to Environment/open is currently ignored in cpython.
 
 2021-04-19 v1.2.1
 * Resolve CI bug where non-Linux wheels were not being published to PyPI.

README.md

@@ -3,11 +3,12 @@ This is a universal Python binding for the LMDB ‘Lightning’ Database.
 See [the documentation](https://lmdb.readthedocs.io) for more information.
 
 ### CI State
-![Build, run, and test py-lmdb](https://github.com/jnwatson/py-lmdb/workflows/Build,%20run,%20and%20test%20py-lmdb/badge.svg)
+[![master](https://github.com/jnwatson/py-lmdb/workflows/Build,%20run,%20and%20test%20py-lmdb/badge.svg)](https://github.com/jnwatson/py-lmdb/actions/workflows/python-package.yml)
 
 # Python Version Support Statement
 
-This project has been around for a while.  Previously, it supported all the way back to before 2.5.  Currently, py-lmdb
-supports Python >= 3.5 and pypy.
+This project has been around for a while.  Previously, it supported all the 
+way back to before Python 2.5.  Currently, py-lmdb supports Python >= 3.5 
+and pypy.
 
 The last version of py-lmdb that supported Python 2.7 was 1.4.1.

lmdb/__init__.py

@@ -50,4 +50,4 @@ def _reading_docs():
     from lmdb.cffi import __all__
     from lmdb.cffi import __doc__
 
-__version__ = '1.6.2'
+__version__ = '1.7.0.dev0'

lmdb/cpython.c

@@ -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
@@ -166,8 +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. */
@@ -1162,7 +1167,7 @@ env_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
         int max_dbs;
         int max_spare_txns;
         int lock;
-    } arg = {NULL, 10485760, 1, 0, 1, 1, 0, 0755, 1, 1, 0, 1, 126, 0, 1, 1};
+    } arg = {NULL, 10485760, 1, 0, 1, 1, 0, 0755, 1, 1, 0, 1, 126, 0, 0, 1};
 
     static const struct argspec argspec[] = {
         {"path", ARG_OBJ, OFFSET(env_new, path)},
@@ -1180,7 +1185,7 @@ env_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
         {"max_readers", ARG_INT, OFFSET(env_new, max_readers)},
         {"max_dbs", ARG_INT, OFFSET(env_new, max_dbs)},
         {"max_spare_txns", ARG_INT, OFFSET(env_new, max_spare_txns)},
-        {"lock", ARG_BOOL, OFFSET(env_new, lock)}
+        {"lock", ARG_BOOL, OFFSET(env_new, lock)},
     };
 
     PyObject *fspath_obj = NULL;
@@ -1208,6 +1213,8 @@ env_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
     self->main_db = NULL;
     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);
@@ -3213,16 +3220,21 @@ trans_dealloc(TransObject *self)
         PyObject_ClearWeakRefs((PyObject *) self);
     }
 
-    if(txn && self->env && !self->env->spare_txn &&
-      (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);
 }
 

tests/env_test.py

@@ -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)

tests/txn_test.py

@@ -1,5 +1,5 @@
 #
-# Copyright 2013 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
@@ -20,8 +20,8 @@
 # <http://www.openldap.org/>.
 #
 
-from __future__ import absolute_import
-from __future__ import with_statement
+import os
+import sys
 import struct
 import unittest
 import weakref
@@ -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()
 
@@ -140,6 +140,39 @@ def test_buffers(self):
         assert isinstance(b, type(B('')))
         txn.abort()
 
+    @unittest.skipIf(sys.platform.startswith('win'), "No fork on Windows")
+    def test_cached_txn_across_fork(self):
+        _, env = testlib.temp_env()
+        txn = env.begin(write=False)
+        del txn
+
+        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'
+        del txn
+
 
 class ContextManagerTest(unittest.TestCase):
     def tearDown(self):