Commit 12d1a05d authored by Kirill Smelkov's avatar Kirill Smelkov Committed by GitHub

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' % <negative-number>`. 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: <unprintable InvalidObjectReference object>

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:

    <Connection at 0x7fef09989d10>

after this patch:

    <ZODB.Connection.Connection object at 0x7fef09989d10>

/reviewed-by @dataflake, @d-maurer 
/reviewed-at https://github.com/zopefoundation/ZODB/pull/384
parent 8a7e3162
...@@ -53,7 +53,6 @@ from ZODB.serialize import ObjectReader ...@@ -53,7 +53,6 @@ from ZODB.serialize import ObjectReader
from ZODB.serialize import ObjectWriter from ZODB.serialize import ObjectWriter
from ZODB.utils import oid_repr from ZODB.utils import oid_repr
from ZODB.utils import p64 from ZODB.utils import p64
from ZODB.utils import positive_id
from ZODB.utils import u64 from ZODB.utils import u64
from ZODB.utils import z64 from ZODB.utils import z64
...@@ -947,15 +946,6 @@ class Connection(ExportImport, object): ...@@ -947,15 +946,6 @@ class Connection(ExportImport, object):
c._cache = PickleCache(self, 0, 0) c._cache = PickleCache(self, 0, 0)
c.close(False) c.close(False)
##########################################################################
# Python protocol
def __repr__(self):
return '<Connection at %08x>' % (positive_id(self),)
# Python protocol
##########################################################################
########################################################################## ##########################################################################
# DEPRECATION candidates # DEPRECATION candidates
......
...@@ -63,7 +63,7 @@ It isn't valid to create references outside a multi database: ...@@ -63,7 +63,7 @@ It isn't valid to create references outside a multi database:
... ...
InvalidObjectReference: InvalidObjectReference:
('Attempt to store an object from a foreign database connection', ('Attempt to store an object from a foreign database connection',
<Connection at ...>, <ZODB.Connection.Connection object at ...>,
<ZODB.tests.testcrossdatabasereferences.MyClass...>) <ZODB.tests.testcrossdatabasereferences.MyClass...>)
>>> tm.abort() >>> tm.abort()
...@@ -92,7 +92,7 @@ reachable from multiple databases: ...@@ -92,7 +92,7 @@ reachable from multiple databases:
InvalidObjectReference: InvalidObjectReference:
("A new object is reachable from multiple databases. Won't try to ("A new object is reachable from multiple databases. Won't try to
guess which one was correct!", guess which one was correct!",
<Connection at ...>, <ZODB.Connection.Connection object at ...>,
<ZODB.tests.testcrossdatabasereferences.MyClass...>) <ZODB.tests.testcrossdatabasereferences.MyClass...>)
>>> tm.abort() >>> tm.abort()
...@@ -120,7 +120,7 @@ This doesn't work with a savepoint: ...@@ -120,7 +120,7 @@ This doesn't work with a savepoint:
InvalidObjectReference: InvalidObjectReference:
("A new object is reachable from multiple databases. Won't try to guess ("A new object is reachable from multiple databases. Won't try to guess
which one was correct!", which one was correct!",
<Connection at ...>, <ZODB.Connection.Connection object at ...>,
<ZODB.tests.testcrossdatabasereferences.MyClass...>) <ZODB.tests.testcrossdatabasereferences.MyClass...>)
>>> tm.abort() >>> tm.abort()
...@@ -167,7 +167,7 @@ the other way around. ...@@ -167,7 +167,7 @@ the other way around.
... ...
InvalidObjectReference: InvalidObjectReference:
("Database '2' doesn't allow implicit cross-database references", ("Database '2' doesn't allow implicit cross-database references",
<Connection at ...>, <ZODB.Connection.Connection object at ...>,
{'x': {}}) {'x': {}})
>>> transaction.abort() >>> transaction.abort()
......
...@@ -86,12 +86,12 @@ You can (still) get a connection to a database this way: ...@@ -86,12 +86,12 @@ You can (still) get a connection to a database this way:
>>> tm = transaction.TransactionManager() >>> tm = transaction.TransactionManager()
>>> cn = db.open(transaction_manager=tm) >>> cn = db.open(transaction_manager=tm)
>>> cn # doctest: +ELLIPSIS >>> cn # doctest: +ELLIPSIS
<Connection at ...> <ZODB.Connection.Connection object at ...>
This is the only connection in this collection right now: This is the only connection in this collection right now:
>>> cn.connections # doctest: +ELLIPSIS >>> cn.connections # doctest: +ELLIPSIS
{'root': <Connection at ...>} {'root': <ZODB.Connection.Connection object at ...>}
Getting a connection to a different database from an existing connection in the Getting a connection to a different database from an existing connection in the
same database collection (this enables 'connection binding' within a given same database collection (this enables 'connection binding' within a given
...@@ -99,7 +99,7 @@ thread/transaction/context ...): ...@@ -99,7 +99,7 @@ thread/transaction/context ...):
>>> cn2 = cn.get_connection('notroot') >>> cn2 = cn.get_connection('notroot')
>>> cn2 # doctest: +ELLIPSIS >>> cn2 # doctest: +ELLIPSIS
<Connection at ...> <ZODB.Connection.Connection object at ...>
The second connection gets the same transaction manager as the first: The second connection gets the same transaction manager as the first:
......
...@@ -61,7 +61,7 @@ database open function, but this doesn't work: ...@@ -61,7 +61,7 @@ database open function, but this doesn't work:
InvalidObjectReference: InvalidObjectReference:
('Attempt to store a reference to an object from a separate connection to ('Attempt to store a reference to an object from a separate connection to
the same database or multidatabase', the same database or multidatabase',
<Connection at ...>, <ZODB.Connection.Connection object at ...>,
<ZODB.tests.testcrossdatabasereferences.MyClass object at ...>) <ZODB.tests.testcrossdatabasereferences.MyClass object at ...>)
>>> tm.abort() >>> tm.abort()
...@@ -80,7 +80,7 @@ different connections to the same database. ...@@ -80,7 +80,7 @@ different connections to the same database.
InvalidObjectReference: InvalidObjectReference:
('Attempt to store a reference to an object from a separate connection ('Attempt to store a reference to an object from a separate connection
to the same database or multidatabase', to the same database or multidatabase',
<Connection at ...>, <ZODB.Connection.Connection object at ...>,
<ZODB.tests.testcrossdatabasereferences.MyClass object at ...>) <ZODB.tests.testcrossdatabasereferences.MyClass object at ...>)
>>> tm.abort() >>> tm.abort()
......
...@@ -41,7 +41,6 @@ __all__ = ['z64', ...@@ -41,7 +41,6 @@ __all__ = ['z64',
'oid_repr', 'oid_repr',
'serial_repr', 'serial_repr',
'tid_repr', 'tid_repr',
'positive_id',
'readable_tid_repr', 'readable_tid_repr',
'get_pickle_metadata', 'get_pickle_metadata',
'locked', 'locked',
...@@ -184,28 +183,6 @@ def readable_tid_repr(tid): ...@@ -184,28 +183,6 @@ def readable_tid_repr(tid):
result = "%s %s" % (result, TimeStamp(tid)) result = "%s %s" % (result, TimeStamp(tid))
return result 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). # Given a ZODB pickle, return pair of strings (module_name, class_name).
# Do this without importing the module or class object. # Do this without importing the module or class object.
# See ZODB/serialize.py's module docstring for the only docs that exist about # See ZODB/serialize.py's module docstring for the only docs that exist about
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment