I was thinking, maybe instead we can add a feature to cffi "don't release the GIL" and use that there? (it would be faster for example)
On Wed, May 6, 2015 at 9:39 PM, arigo <nore...@buildbot.pypy.org> wrote: > Author: Armin Rigo <ar...@tunes.org> > Branch: > Changeset: r77171:df44050e8e33 > Date: 2015-05-06 21:39 +0200 > http://bitbucket.org/pypy/pypy/changeset/df44050e8e33/ > > Log: The gdbm library is not thread-safe. Add a global lock. > > diff --git a/lib_pypy/gdbm.py b/lib_pypy/gdbm.py > --- a/lib_pypy/gdbm.py > +++ b/lib_pypy/gdbm.py > @@ -1,4 +1,6 @@ > import cffi, os, sys > +import thread > +_lock = thread.allocate_lock() > > ffi = cffi.FFI() > ffi.cdef(''' > @@ -40,6 +42,7 @@ > > try: > verify_code = ''' > + #include <stdlib.h> > #include "gdbm.h" > > static datum pygdbm_fetch(GDBM_FILE gdbm_file, char *dptr, int dsize) { > @@ -86,102 +89,121 @@ > return {'dptr': ffi.new("char[]", key), 'dsize': len(key)} > > class gdbm(object): > - ll_dbm = None > + __ll_dbm = None > + > + # All public methods need to acquire the lock; all private methods > + # assume the lock is already held. Thus public methods cannot call > + # other public methods. > > def __init__(self, filename, iflags, mode): > - res = lib.gdbm_open(filename, 0, iflags, mode, ffi.NULL) > - self.size = -1 > - if not res: > - self._raise_from_errno() > - self.ll_dbm = res > + with _lock: > + res = lib.gdbm_open(filename, 0, iflags, mode, ffi.NULL) > + self.__size = -1 > + if not res: > + self.__raise_from_errno() > + self.__ll_dbm = res > > def close(self): > - if self.ll_dbm: > - lib.gdbm_close(self.ll_dbm) > - self.ll_dbm = None > + with _lock: > + if self.__ll_dbm: > + lib.gdbm_close(self.__ll_dbm) > + self.__ll_dbm = None > > - def _raise_from_errno(self): > + def __raise_from_errno(self): > if ffi.errno: > raise error(ffi.errno, os.strerror(ffi.errno)) > raise error(lib.gdbm_errno, lib.gdbm_strerror(lib.gdbm_errno)) > > def __len__(self): > - if self.size < 0: > - self.size = len(self.keys()) > - return self.size > + with _lock: > + if self.__size < 0: > + self.__size = len(self.__keys()) > + return self.__size > > def __setitem__(self, key, value): > - self._check_closed() > - self.size = -1 > - r = lib.gdbm_store(self.ll_dbm, _fromstr(key), _fromstr(value), > - lib.GDBM_REPLACE) > - if r < 0: > - self._raise_from_errno() > + with _lock: > + self.__check_closed() > + self.__size = -1 > + r = lib.gdbm_store(self.__ll_dbm, _fromstr(key), _fromstr(value), > + lib.GDBM_REPLACE) > + if r < 0: > + self.__raise_from_errno() > > def __delitem__(self, key): > - self._check_closed() > - self.size = -1 > - res = lib.gdbm_delete(self.ll_dbm, _fromstr(key)) > - if res < 0: > - raise KeyError(key) > + with _lock: > + self.__check_closed() > + self.__size = -1 > + res = lib.gdbm_delete(self.__ll_dbm, _fromstr(key)) > + if res < 0: > + raise KeyError(key) > > def __contains__(self, key): > - self._check_closed() > - key = _checkstr(key) > - return lib.pygdbm_exists(self.ll_dbm, key, len(key)) > + with _lock: > + self.__check_closed() > + key = _checkstr(key) > + return lib.pygdbm_exists(self.__ll_dbm, key, len(key)) > has_key = __contains__ > > def __getitem__(self, key): > - self._check_closed() > - key = _checkstr(key) > - drec = lib.pygdbm_fetch(self.ll_dbm, key, len(key)) > - if not drec.dptr: > - raise KeyError(key) > - res = str(ffi.buffer(drec.dptr, drec.dsize)) > - lib.free(drec.dptr) > - return res > + with _lock: > + self.__check_closed() > + key = _checkstr(key) > + drec = lib.pygdbm_fetch(self.__ll_dbm, key, len(key)) > + if not drec.dptr: > + raise KeyError(key) > + res = str(ffi.buffer(drec.dptr, drec.dsize)) > + lib.free(drec.dptr) > + return res > > - def keys(self): > - self._check_closed() > + def __keys(self): > + self.__check_closed() > l = [] > - key = lib.gdbm_firstkey(self.ll_dbm) > + key = lib.gdbm_firstkey(self.__ll_dbm) > while key.dptr: > l.append(str(ffi.buffer(key.dptr, key.dsize))) > - nextkey = lib.gdbm_nextkey(self.ll_dbm, key) > + nextkey = lib.gdbm_nextkey(self.__ll_dbm, key) > lib.free(key.dptr) > key = nextkey > return l > > + def keys(self): > + with _lock: > + return self.__keys() > + > def firstkey(self): > - self._check_closed() > - key = lib.gdbm_firstkey(self.ll_dbm) > - if key.dptr: > - res = str(ffi.buffer(key.dptr, key.dsize)) > - lib.free(key.dptr) > - return res > + with _lock: > + self.__check_closed() > + key = lib.gdbm_firstkey(self.__ll_dbm) > + if key.dptr: > + res = str(ffi.buffer(key.dptr, key.dsize)) > + lib.free(key.dptr) > + return res > > def nextkey(self, key): > - self._check_closed() > - key = lib.gdbm_nextkey(self.ll_dbm, _fromstr(key)) > - if key.dptr: > - res = str(ffi.buffer(key.dptr, key.dsize)) > - lib.free(key.dptr) > - return res > + with _lock: > + self.__check_closed() > + key = lib.gdbm_nextkey(self.__ll_dbm, _fromstr(key)) > + if key.dptr: > + res = str(ffi.buffer(key.dptr, key.dsize)) > + lib.free(key.dptr) > + return res > > def reorganize(self): > - self._check_closed() > - if lib.gdbm_reorganize(self.ll_dbm) < 0: > - self._raise_from_errno() > + with _lock: > + self.__check_closed() > + if lib.gdbm_reorganize(self.__ll_dbm) < 0: > + self.__raise_from_errno() > > - def _check_closed(self): > - if not self.ll_dbm: > + def __check_closed(self): > + if not self.__ll_dbm: > raise error(0, "GDBM object has already been closed") > > __del__ = close > > def sync(self): > - self._check_closed() > - lib.gdbm_sync(self.ll_dbm) > + with _lock: > + self.__check_closed() > + lib.gdbm_sync(self.__ll_dbm) > > def open(filename, flags='r', mode=0666): > if flags[0] == 'r': > _______________________________________________ > pypy-commit mailing list > pypy-com...@python.org > https://mail.python.org/mailman/listinfo/pypy-commit _______________________________________________ pypy-dev mailing list pypy-dev@python.org https://mail.python.org/mailman/listinfo/pypy-dev
