From 12d1a05d1e2a4ccaf0ee205a1baa61db08607e11 Mon Sep 17 00:00:00 2001 From: Kirill Smelkov Date: Fri, 26 May 2023 16:43:29 +0300 Subject: [PATCH] Remove potentially-buggy positive_id (#384) positive_id was added in 2004 in f7b96aea (ZODB.utils grows a new function positive_id(), which returns the id() of an object as a non-negative integer ...) to workaround change in behaviour Python2.3 -> 2.4 for `'%x' % `. And nowdays `positive_id` is used only in one place in Connection.__repr__ to mimic approximately what Python already gives for a default __repr__ out of the box. On https://github.com/zopefoundation/ZODB/actions/runs/5067242751/jobs/9098062772 we recently hit ZODB.POSException.InvalidObjectReference: for the following code: if self._jar.get_connection(database_name) is not obj._p_jar: raise InvalidObjectReference( "Attempt to store a reference to an object from " "a separate connection to the same database or " "multidatabase", self._jar, obj, ) ( https://github.com/zopefoundation/ZODB/blob/5.8.0-24-g8a7e3162d/src/ZODB/serialize.py#L366-L371 ) for which the "unprintable ..." part is produced by `Lib/traceback.py` who tries to do `str(ex)` and returns `unprintable ...` if that str failed: https://github.com/python/cpython/blob/v3.8.16-18-g9f89c471fb1/Lib/traceback.py#L153-L157 Since one of the reasons for that failure might have been a bit non-trivial code and assert in positive_id, let's remove it and rely on builtin python behaviour for Connection repr. repr for Connection: before this patch: after this patch: /reviewed-by @dataflake, @d-maurer /reviewed-at https://github.com/zopefoundation/ZODB/pull/384 --- src/ZODB/Connection.py | 10 -------- src/ZODB/cross-database-references.rst | 8 +++---- src/ZODB/tests/multidb.txt | 6 ++--- src/ZODB/tests/testcrossdatabasereferences.py | 4 ++-- src/ZODB/utils.py | 23 ------------------- 5 files changed, 9 insertions(+), 42 deletions(-) diff --git a/src/ZODB/Connection.py b/src/ZODB/Connection.py index c067aa1e9..255f8438a 100644 --- a/src/ZODB/Connection.py +++ b/src/ZODB/Connection.py @@ -53,7 +53,6 @@ from ZODB.serialize import ObjectWriter from ZODB.utils import oid_repr from ZODB.utils import p64 -from ZODB.utils import positive_id from ZODB.utils import u64 from ZODB.utils import z64 @@ -947,15 +946,6 @@ def _release_resources(self): c._cache = PickleCache(self, 0, 0) c.close(False) - ########################################################################## - # Python protocol - - def __repr__(self): - return '' % (positive_id(self),) - - # Python protocol - ########################################################################## - ########################################################################## # DEPRECATION candidates diff --git a/src/ZODB/cross-database-references.rst b/src/ZODB/cross-database-references.rst index 6690d45b0..b5e043bc8 100644 --- a/src/ZODB/cross-database-references.rst +++ b/src/ZODB/cross-database-references.rst @@ -63,7 +63,7 @@ It isn't valid to create references outside a multi database: ... InvalidObjectReference: ('Attempt to store an object from a foreign database connection', - , + , ) >>> tm.abort() @@ -92,7 +92,7 @@ reachable from multiple databases: InvalidObjectReference: ("A new object is reachable from multiple databases. Won't try to guess which one was correct!", - , + , ) >>> tm.abort() @@ -120,7 +120,7 @@ This doesn't work with a savepoint: InvalidObjectReference: ("A new object is reachable from multiple databases. Won't try to guess which one was correct!", - , + , ) >>> tm.abort() @@ -167,7 +167,7 @@ the other way around. ... InvalidObjectReference: ("Database '2' doesn't allow implicit cross-database references", - , + , {'x': {}}) >>> transaction.abort() diff --git a/src/ZODB/tests/multidb.txt b/src/ZODB/tests/multidb.txt index e8dffcfec..78fea006f 100644 --- a/src/ZODB/tests/multidb.txt +++ b/src/ZODB/tests/multidb.txt @@ -86,12 +86,12 @@ You can (still) get a connection to a database this way: >>> tm = transaction.TransactionManager() >>> cn = db.open(transaction_manager=tm) >>> cn # doctest: +ELLIPSIS - + This is the only connection in this collection right now: >>> cn.connections # doctest: +ELLIPSIS - {'root': } + {'root': } Getting a connection to a different database from an existing connection in the same database collection (this enables 'connection binding' within a given @@ -99,7 +99,7 @@ thread/transaction/context ...): >>> cn2 = cn.get_connection('notroot') >>> cn2 # doctest: +ELLIPSIS - + The second connection gets the same transaction manager as the first: diff --git a/src/ZODB/tests/testcrossdatabasereferences.py b/src/ZODB/tests/testcrossdatabasereferences.py index 3f1bbde6b..c6b96176e 100644 --- a/src/ZODB/tests/testcrossdatabasereferences.py +++ b/src/ZODB/tests/testcrossdatabasereferences.py @@ -61,7 +61,7 @@ def test_must_use_consistent_connections(): InvalidObjectReference: ('Attempt to store a reference to an object from a separate connection to the same database or multidatabase', - , + , ) >>> tm.abort() @@ -80,7 +80,7 @@ def test_must_use_consistent_connections(): InvalidObjectReference: ('Attempt to store a reference to an object from a separate connection to the same database or multidatabase', - , + , ) >>> tm.abort() diff --git a/src/ZODB/utils.py b/src/ZODB/utils.py index 566b98f5a..ba46d0c11 100644 --- a/src/ZODB/utils.py +++ b/src/ZODB/utils.py @@ -41,7 +41,6 @@ 'oid_repr', 'serial_repr', 'tid_repr', - 'positive_id', 'readable_tid_repr', 'get_pickle_metadata', 'locked', @@ -184,28 +183,6 @@ def readable_tid_repr(tid): result = "%s %s" % (result, TimeStamp(tid)) return result -# Addresses can "look negative" on some boxes, some of the time. If you -# feed a "negative address" to an %x format, Python 2.3 displays it as -# unsigned, but produces a FutureWarning, because Python 2.4 will display -# it as signed. So when you want to prodce an address, use positive_id() to -# obtain it. -# _ADDRESS_MASK is 2**(number_of_bits_in_a_native_pointer). Adding this to -# a negative address gives a positive int with the same hex representation as -# the significant bits in the original. - - -_ADDRESS_MASK = 256 ** struct.calcsize('P') - - -def positive_id(obj): - """Return id(obj) as a non-negative integer.""" - - result = id(obj) - if result < 0: - result += _ADDRESS_MASK - assert result > 0 - return result - # Given a ZODB pickle, return pair of strings (module_name, class_name). # Do this without importing the module or class object. # See ZODB/serialize.py's module docstring for the only docs that exist about