bytes size estimate in cPickleCache can overflow

Bug #533015 reported by Hanno Schlichting on 2010-03-05
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
ZODB
Undecided
Unassigned

Bug Description

The cc_update_object_size_estimation uses an unsigned int, where it should use an int. This can lead to overflow errors where the estimated cache size can suddenly become negative and overflowing to "4294967296". After that happens the cache will always evict all objects from the cache, as it estimates it's already more than full.

Hanno Schlichting (hannosch) wrote :
Hanno Schlichting (hannosch) wrote :

The patch is applied to a branch made from trunk at svn+ssh://svn.zope.org/repos/main/ZODB/branches/hannosch-lp533015-bytesizecache

Hanno Schlichting (hannosch) wrote :

The test fails without the one line patch and works with it. This was tested on Mac OS 10.6 with a 64bit Python 2.6

Hanno Schlichting (hannosch) wrote :

Verified the patch / test on both:

Python 2.6.4 (r264:75706, Jan 11 2010, 16:25:19)
[GCC 4.2.1 (Apple Inc. build 5646)] on darwin

and

Python 2.4.6 (#1, Jan 11 2010, 16:18:54)
[GCC 4.2.1 (Apple Inc. build 5646)] on darwin

Nikolay Kim (fafhrd91) wrote :

verified patch

Python 2.4.4 (#71, Oct 18 2006, 08:34:43) [MSC v.1310 32 bit (Intel)] on win32
gcc-3.4.5 (mingw)

but Hanno's test passed

Nikolay Kim (fafhrd91) wrote :

this test raises assertion, tested on python2.6 python2.4 32-bin linux
without patch total_estimated_size is set to 0, and cache size is 1 (at this point cache is cleaned up on each collecting garbage)
with patch total_estimated_size is set to 320 and cache size is 2

    def test_cache_garbage_collection_shrinking_object(self):
        db = self.db
        # activate size based cache garbage collection
        db.setCacheSizeBytes(1000)
        obj, conn, cache = self.obj, self.conn, self.conn._cache
        # verify the change worked as expected
        self.assertEqual(cache.cache_size_bytes, 1000)
        # verify our entrance assumption is fullfilled
        self.assert_(cache.total_estimated_size > 1)
        # give the objects some size
        obj.setValueWithSize(500)
        transaction.savepoint()
        self.assert_(cache.total_estimated_size > 500)
        # make the object smaller
        obj.setValueWithSize(100)
        transaction.savepoint()
        # make sure there was no overflow
        self.assert_(cache.total_estimated_size != 0)

Hanno Schlichting (hannosch) wrote :

@Nikolay: thx for the extra tests

I committed a modified version of that as:

# the object still fits into the cache, so it should be there
self.assert_(cache.total_estimated_size >= 100)

but retained the old test for <= 1000. Depending on the platform the value was apparently either 0 or "-1" expressed at 2**32, so we need to test both conditions.

Changed in zodb:
status: New → Fix Committed
Tres Seaver (tseaver) wrote :
Changed in zodb:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers