[Zope3-checkins] CVS: ZODB3/ZODB - cPickleCache.c:1.85.6.4

[Chris McDonough]

This suspense is killing me! ;-)

On Tue, 2004-05-18 at 14:31, Tim Peters wrote:
> Update of /cvs-repository/ZODB3/ZODB
> In directory cvs.zope.org:/tmp/cvs-serv6721/ZODB
> 
> Modified Files:
>       Tag: Zope-2_7-branch
> 	cPickleCache.c 
> Log Message:
> scan_gc_items():  so-far semantically neutral code and comment cleanups.
> Gave it a single exit point because it will soon need one <wink>.  Added
> simple insert_after() and unlink_from_ring() utility functions (which
> will also simplify what's coming next).
> 
> 
> === ZODB3/ZODB/cPickleCache.c 1.85.6.3 => 1.85.6.4 ===
> --- ZODB3/ZODB/cPickleCache.c:1.85.6.3	Tue May 18 11:54:44 2004
> +++ ZODB3/ZODB/cPickleCache.c	Tue May 18 14:31:24 2004
> @@ -146,6 +146,27 @@
>  
>  } ccobject;
>  
> +/* Insert self into the ring, following after. */
> +static void
> +insert_after(CPersistentRing *self, CPersistentRing *after)
> +{
> +    assert(self != NULL);
> +    assert(after != NULL);
> +    self->prev = after;
> +    self->next = after->next;
> +    after->next->prev = self;
> +    after->next = self;
> +}
> +
> +/* Remove self from the ring. */
> +static void
> +unlink_from_ring(CPersistentRing *self)
> +{
> +    assert(self != NULL);
> +    self->prev->next = self->next;
> +    self->next->prev = self->prev;
> +}
> +
>  static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v);
>  
>  /* ---------------------------------------------------------------- */
> @@ -159,20 +180,18 @@
>      /* This function must only be called with the ring lock held,
>         because it places a non-object placeholder in the ring.
>      */
> -
>      cPersistentObject *object;
>      int error;
>      CPersistentRing placeholder;
>      CPersistentRing *here = self->ring_home.next;
> +    int result = -1;   /* guilty until proved innocent */
>  
>      /* Scan through the ring until we either find the ring_home (i.e. start
> -     * of the ring, or we've ghosted enough objects to reach the target
> +     * of the ring), or we've ghosted enough objects to reach the target
>       * size.
>       */
> -    while (1) {
> -	/* back to the home position. stop looking */
> -        if (here == &self->ring_home)
> -            return 0;
> +    while (here != &self->ring_home && self->non_ghost_count > target) {
> +	assert(self->ring_lock);
>  
>          /* At this point we know that the ring only contains nodes
>  	   from persistent objects, plus our own home node. We know
> @@ -180,51 +199,40 @@
>  	   the current ring node is a persistent object now we know it
>  	   is not the home */
>          object = OBJECT_FROM_RING(self, here, "scan_gc_items");
> -        if (!object)
> -	    return -1;
>  
> -	/* we are small enough */
> -        if (self->non_ghost_count <= target)
> -            return 0;
> -        else if (object->state == cPersistent_UPTODATE_STATE) {
> +        if (object->state == cPersistent_UPTODATE_STATE) {
>              /* deactivate it. This is the main memory saver. */
>  
> -            /* Add a placeholder; a dummy node in the ring.  We need
> +            /* Add a placeholder;  a dummy node in the ring.  We need
>  	       to do this to mark our position in the ring.  It is
>  	       possible that the PyObject_SetAttr() call below will
> -	       invoke an __setattr__() hook in Python.  If it does,
> -	       another thread might run; if that thread accesses a
> -	       persistent object and moves it to the head of the ring,
> -	       it might cause the gc scan to start working from the
> -	       head of the list.
> +	       invoke a __setattr__() hook in Python.  Also possible
> +	       that deactivation will lead to a __del__ method call.
> +	       So another thread might run, and mutate the ring as a side
> +	       effect of object accesses.  There's no predicting then where
> +	       in the ring here->next will point after that.  The
> +	       placeholder won't move as a side effect of calling Python
> +	       code.
>  	    */
> -
> -            placeholder.next = here->next;
> -            placeholder.prev = here;
> -            here->next->prev = &placeholder;
> -            here->next = &placeholder;
> -
> +            insert_after(&placeholder, here);
>              ENGINE_NOISE("G");
>  
> -            /* In Python, "obj._p_changed = None" spells, ghostify */
> +            /* In Python, "obj._p_changed = None" spells ghostify */
>              error = PyObject_SetAttr((PyObject *)object, py__p_changed,
>  				     Py_None);
> -
> -
> -            /* unlink the placeholder */
> -            placeholder.next->prev = placeholder.prev;
> -            placeholder.prev->next = placeholder.next;
> -
>              here = placeholder.next;
> -
> -            if (error)
> -                return -1; /* problem */
> +            unlink_from_ring(&placeholder);
> +            if (error < 0)
> +                goto Done;
>          }
>          else {
>              ENGINE_NOISE(".");
>              here = here->next;
>          }
>      }
> +    result = 0;
> + Done:
> +    return result;
>  }
>  
>  static PyObject *
> @@ -241,7 +249,7 @@
>  
>      ENGINE_NOISE("<");
>      self->ring_lock = 1;
> -    if (scan_gc_items(self, target_size)) {
> +    if (scan_gc_items(self, target_size) < 0) {
>          self->ring_lock = 0;
>          return NULL;
>      }
> 
> 
> _______________________________________________
> Zope3-Checkins mailing list
> Zope3-Checkins at zope.org
> http://mail.zope.org/mailman/listinfo/zope3-checkins