A list for the developers of CellML tools
Text archives Help
- From: r.britten at auckland.ac.nz (Randall Britten)
- Subject: [cellml-dev] r2914 - CellML_DOM_API/trunk/CUSES/sources
- Date: Tue, 18 Nov 2008 15:23:16 +1300
Tracker item 1490
> -----Original Message-----
> From: cellml-tools-developers-bounces at cellml.org [mailto:cellml-tools-
> developers-bounces at cellml.org] On Behalf Of Andrew Miller
> Sent: Tuesday, 18 November 2008 3:17 p.m.
> To: cellml-tools-developers at cellml.org
> Subject: Re: [cellml-dev] r2914 - CellML_DOM_API/trunk/CUSES/sources
>
> CellML Automated Notifications wrote:
> > Author: jmar150
> > Date: 2008-11-18 14:14:04 +1300 (Tue, 18 Nov 2008)
> > New Revision: 2914
> >
> > Modified:
> > CellML_DOM_API/trunk/CUSES/sources/CUSESImpl.cpp
> > Log:
> > Fix a segmentation fault when a unit is canonicalised into five base
> units,
> > with the three in the middle having the same base unit but different
> prefixes
> > or exponents, and the resulting exponent is non-zero.
> >
> Hi all,
>
> Do we have a tracker item for this? I'm not convinced the fix is
> correct
> - I suspect it will leak in some (most?) cases. As is convention in the
> CellML API implementation, after the new call, the refcount is one, and
> then the add_ref boosts the reference count to two. The
> BaseUnitInstance
> with refcount of two is then added on to newBaseUnits. uThis and uLast
> are only temporary pointers which rely on the assumption that the
> objects will outlive the code because there is a reference on either
> baseUnits or newBaseUnits, and so at the end of that code block, the
> only reference to the new BaseUnitInstance is on the newBaseUnits list,
> and yet the refcount is two.
>
> It is possible that having an artificially high refcount works around a
> bug somewhere else in the code where we drop the refcount by one too
> many, and therefore works around the problem, but I don't think this is
> the proper solution, and when we don't hit the bug that drops the
> refcount by one too many, we will leak.
>
> I don't mind looking into this in more depth tomorrow, but it would be
> good to have a tracker item on it, with a model which reproduces the
> bug, which I presume someone has?
>
> Best wishes,
> Andrew
>
> >
> > Modified: CellML_DOM_API/trunk/CUSES/sources/CUSESImpl.cpp
> > ===================================================================
> > --- CellML_DOM_API/trunk/CUSES/sources/CUSESImpl.cpp 2008-11-18
> 00:53:03 UTC (rev 2913)
> > +++ CellML_DOM_API/trunk/CUSES/sources/CUSESImpl.cpp 2008-11-18
> 01:14:04 UTC (rev 2914)
> > @@ -283,6 +283,7 @@
> > if (newExp != 0)
> > {
> > uLast = new CDABaseUnitInstance(buThis, newPref, 0.0,
> newExp);
> > + uLast->add_ref();
> > newBaseUnits.push_back(uLast);
> > }
> > continue;
> >
> > _______________________________________________
> > automated-notifications mailing list
> > automated-notifications at cellml.org
> > http://www.cellml.org/mailman/listinfo/automated-notifications
> >
>
> _______________________________________________
> cellml-tools-developers mailing list
> cellml-tools-developers at cellml.org
> http://www.cellml.org/mailman/listinfo/cellml-tools-developers
- [cellml-dev] r2914 - CellML_DOM_API/trunk/CUSES/sources, Andrew Miller, 11/18/2008
- [cellml-dev] r2914 - CellML_DOM_API/trunk/CUSES/sources, Randall Britten, 11/18/2008
Archive powered by MHonArc 2.6.18.
