[asterisk-dev] [svn-commits] tilghman: branch tilghman/res_odbc_astobj2 r101392 - /team/tilghman/res_odbc_...

[Tilghman Lesher]

Much of the criticisms were good, but the following corrections are based on
misunderstandings of the intent of the code.

On Thursday 31 January 2008 22:57:25 Russell Bryant wrote:
> > @@ -411,9 +436,7 @@
> >  {
> >  	struct odbc_obj *obj;
> >  	if (class) {
> > -		AST_LIST_LOCK(&odbc_list);
> > -		AST_LIST_INSERT_HEAD(&odbc_list, class, list);
> > -		AST_LIST_UNLOCK(&odbc_list);
> > +		ao2_link(class_container, class);
>
> It looks like you're missing an unreference here.

It's not.  The reference is inherited by the object.

> >  			} else {
> >  				obj->used = 1;
> > -				AST_LIST_INSERT_TAIL(&class->odbc_obj, obj, list);
> > +				ao2_link(class->obj_container, obj);
> >  			}
>
> Looks like you're leaking a reference here, too.  Don't forget the
> change that we made such that ao2_link() automatically increasing the
> reference count.  It no longer inherits a reference.

Right, but I'm doing a circular reference here.  The class inherits the
referenct to the object.

> > +	/* Purge remaining classes */
> > +	aoi = ao2_iterator_init(class_container, OBJ_UNLINK);
> > +	while ((class = ao2_iterator_next(&aoi))) {
> > +		if (class->delme) {
> > +			struct ao2_iterator aoi2 = ao2_iterator_init(class->obj_container,
> > OBJ_UNLINK); +			while ((current = ao2_iterator_next(&aoi2))) {
> > +				ao2_ref(current, -2);
> > +			}
> > +			ao2_ref(class, -1);
> > +		}
> > +		ao2_ref(class, -1);
> > +	}
>
> This loop seems is concerning for a number of reasons.  Really, you
> should be able to simply unref to top level container and everything
> should go away from that.  It also fixes the mem leak of
> class_container.  :)

I need the circular reference to ensure that when there's an outstanding
odbc_obj, it doesn't dispose of the class until the final obj is released.

The class container is never released, as the module is not unloadable
(or else it could break things like func_odbc or app_voicemail).

-- 
Tilghman