Opened 16 years ago

Closed 3 years ago

#331 closed defect (fixed)

Serious bug in FdoPtr<T> copy ctor (patch attached)

Reported by: Kosta Owned by: traianstanev
Priority: blocker Milestone:
Component: FDO API Version: 3.3.1
Severity: 1 Keywords:
Cc: External ID:

Description

If you assign an FdoPtr<T> object "t" to a new FdoPtr<U> object "u" where U is a base class of T the ref count is not increased. Example:

FdoPtr<FdoIPolygon> poly = FdoFgfGeometryFactory::GetInstance()
    ->CreatePolygon(outerRing, innerRings);
// ref count of poly == 1

FdoPtr<FdoIGeometry> geom = poly;
// ref count of poly (and geom) still == 1

If both pointers go out of scope the pointed to object gets deleted by the first FdoPtr-dtor and the second pointer holds a dangling pointer resulting in a crash during destruction...

The problem is that FdoPtr<T> only defines a copy ctor for type T. So during construction of the "geom" pointer, first "FdoPtr<FdoIPolygon>::operator FdoIPolygon*()" is called on the "poly" pointer and the resulting plain "FdoIPolygon*" pointer is feed into "FdoPtr<FdoIGeometry>::FdoPtr(FdoIGeometry* t)"; but this ctor does not increase the reference count of the pointed to object!

Proposed patch:

Change the existing impl of the copy ctor from:

    /// \brief
    /// FdoPtr copy constructor. Wraps a new FdoPtr around the object
    /// referenced by lp.
    /// 
    /// \param lp 
    /// Input the FdoPtr to copy from.
    /// 
    /// \return
    /// Returns a FdoPtr
    /// 
    /// \note
    /// This operator adds a reference on the object.
    FdoPtr(const FdoPtr<T>& lp) throw()
    {
        p = lp.p;
        if (p != NULL)
            p->AddRef();
    }

into:

    /// \brief
    /// FdoPtr copy constructor. Wraps a new FdoPtr around the object
    /// referenced by lp. Type U must be convertible to type T.     <<<<< here
    /// 
    /// \param lp 
    /// Input the FdoPtr to copy from.
    /// 
    /// \return
    /// Returns a FdoPtr
    /// 
    /// \note
    /// This operator adds a reference on the object.
    template <class U>                                              <<<<< here
    FdoPtr(const FdoPtr<U>& lp) throw()                             <<<<< here
    {
        p = lp.p;
        if (p != NULL)
            p->AddRef();
    }

Change History (10)

comment:1 by Kosta, 16 years ago

Correction: at least for VS2005 there must be both ctors available...

comment:2 by gregboone, 16 years ago

Status: newassigned

comment:3 by gregboone, 16 years ago

Resolution: wontfix
Status: assignedclosed

The FDO teams prefers not too change this behaviour in 3.4.0. It is a bit late in the development cycle and we cannot be sure of the resulting impact on all our provider and application implemnetations. I am aware that workaround have been made in application due to this issue and there is not enough time to identify and resolve all those at this time.

Defer to 3.5 or 4.0.

comment:4 by gregboone, 16 years ago

Resolution: wontfix
Status: closedreopened

comment:5 by gregboone, 16 years ago

Milestone: 3.4.0
Resolution: fixed
Status: reopenedclosed

comment:6 by gregboone, 16 years ago

Instead of closing this issue, let's defer this to a future release. i.e. FDO 4.0

comment:7 by gregboone, 16 years ago

Resolution: fixed
Status: closedreopened

comment:8 by Kosta, 16 years ago

Just a short note: to spot the affected source code lines, it should be enough to put the proposed additional template ctor into the private section of the class declaration and do a recompile...

comment:9 by traianstanev, 15 years ago

Owner: changed from gregboone to traianstanev
Status: reopenednew

I'm picking this up, I intend to fix it together with the removal of the null check from the operator->.

comment:10 by jng, 3 years ago

Resolution: fixed
Status: newclosed

Fixed in r4788

Note: See TracTickets for help on using tickets.