Thanks for your answer Robert,
My test app was clearly a misuse of ref_ptr.
So, I retested it with observer_ptr instead of C pointer.
With the head revision, I haven't been able to reproduce the warning thanks
to this test in the ObserverSet::addRefLock() function :
if (refCount == 1)
{
// The object is in the process of being deleted, but our
// objectDeleted() method hasn't been run yet (and we're
// blocking it -- and the final destruction -- with our lock).
_observedObject->unref_nodelete();
return 0;
}
When assigning an observer_ptr to a ref_ptr in my second thread, the
simultaneous destruction of the pointed object is well checked and then the
refCount is not incremented.
The ref_ptr is set to NULL. All works fine.
With the 2.9.5 version, there was no ref_ptr constructor taking an
observer_ptr argument.
So we have to be careful also when using observer_ptr. It could lead to
misuse of ref_ptr too, since we need to use the observed C pointer to
assign it to ref_ptr.
Indeed, the warning is reproduced with the following assignment in my
test: _imageRefPtr = _imageObserverPtr.get();
Thanks again
2012/2/7 Robert Osfield <[email protected]>
> HI Mikael,
>
> Thanks for the details.
>
> Your comment about the delete not being mutexed made me think about
> why the ref count could get to zero and then have the thread progress
> to delete, and one shouldn't progress to delete as long as the ref
> count is non zero. This led me to wonder just how with your code this
> could happen and I think the crux of it is that in your test app you
> are creating an object using a C pointer to it, then letting different
> threads assign and reset ref_ptr<> independently.
>
> For the case where it crashes my guess is that the main thread creates
> the MyThread and this stores a C pointer to the Image, then the main
> thread calls startThread() which returns to main which then assigns
> and resets the ref_ptr<>, if this assign/reset happens before the
> MyThread::run() gets going and creates it's own ref to the Image then
> it'll be deleted before it gets a chance to increment the ref count.
> This isn't a bug in ref counting, it's a bug in your code - MyThread
> should never have been written with a C pointer in the first place, as
> this opens the door to the sequence I've explain and problem you are
> seeing.
>
> So I think this example is good one for demonstrating how dangerous it
> is to use C pointers and ref_ptr<>'s without taking proper care about
> sequencing of the creation and assignments. These types of errors are
> very hard to spot, even on an example as small of yours it's not
> obvious, so taking a full scale app it becomes very hard to spot these
> problems indeed. The best defence is to be very careful anytime you
> pass C pointers around, and especially carefully when you retain C
> pointers for use later.
>
> Robert.
>
>
> On 7 February 2012 13:50, mikael lemercier <[email protected]>
> wrote:
> >
> > 2012/2/7 Robert Osfield <[email protected]>
> >>
> >> Also what hardware, OS, dev environment are you working on?
> >
> >
> > I'm working on:
> > - Hardware : Intel Core i7-2600
> > - OS : Windows 7
> > - Dev environment : Visual 2010
> > - Build config : x64
> >
> > I also reproduced the problem on:
> > - Hardware : Intel Core i7-2600
> > - OS : OpenSuse 11.1
> > - Dev environment : Eclipse
> >
> >
> >>
> >> Could you also check whether the OSG is using atomic ref counting or
> >> OpenThreads::Mutex version? include/OpenThreads/Config will reveal
> >> which should be being used.
> >
> >
> > It's using atomic ref counting in my case :
> > _OSG_REFERENCED_USE_ATOMIC_OPERATIONS is defined.
> > But the problem might also exist with Mutex, since in both cases the
> delete
> > part is not locked.
> >
> >>
> >> Could you also outline how you are running your tests.
> >
> >
> > To reproduce the problem, without any interference from my work, I
> inserted
> > my test file directly in the osg solution.
> > I just replaced osgviewer.cpp by my test file (joined with my first
> > message).
> > As I said before, I also added a sleep() of 1s to the Referenced::unref()
> > function so that the created thread has time to increment the refCount
> > before the main thread calls the delete.
> >
> >
> >
> >
> >
> >
> >
> > _______________________________________________
> > osg-users mailing list
> > [email protected]
> >
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
> >
> _______________________________________________
> osg-users mailing list
> [email protected]
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
_______________________________________________
osg-users mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
