Chronological Thread 

< Chronological >      < Thread >

---

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-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.