[Mesa-dev] [PATCH] clover: Increment an object's reference count in ret_object()

[Tom Stellard]

On Thu, Jun 28, 2012 at 01:09:20AM +0200, Francisco Jerez wrote:
> Tom Stellard <tstellar at gmail.com> writes:
> 
> > We need to increment the reference count for objects, like cl_event,
> > that the user is responsible for destroying when they are returned from
> > the API.  Otherwise, the object will be destroyed when clover is done with
> > it, even though the user will still have a reference to it.  For example:
> >
> > 1. clEnqueueNDRangeKernel(queue, ... , &event)
> >   - create an event object
> >   - refcount = 1
> >
> > 2. clFlush(queue)
> >   - event object is removed from the queue and its reference count is
> >     decremented.
> >   - refcount = 0, event is deleted
> >
> > 3. clGetEventInfo(event, ...)
> >   - segfault
> 
> I don't think this could cause the problem you've seen...  After step 1
> the event object ends up queued in queue->queued_events, a ref_ptr<>
> list that holds additional references to each object it contains, so,
> after step 1 refcount is supposed to be 2 already...  You're probably
> hitting something more subtle, try the attached patch.
>

I missed that part about ref_ptr.  I'll have to look at that again.
However that sequence is causing a segfault in clGetEventInfo in all
of the AMD OpenCL SDK examples (or at least all of them that make it
that far), and it is because the event is being deleted too early.

-Tom

 
> PS: Can you CC me when you send clover-related patches to the mailing
>     list?  I'm likely to miss them otherwise.
> 
> > ---
> >  src/gallium/state_trackers/clover/api/util.hpp |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/api/util.hpp b/src/gallium/state_trackers/clover/api/util.hpp
> > index 2f9ec1f..5cfc528 100644
> > --- a/src/gallium/state_trackers/clover/api/util.hpp
> > +++ b/src/gallium/state_trackers/clover/api/util.hpp
> > @@ -156,8 +156,10 @@ namespace clover {
> >     template<typename T, typename S>
> >     void
> >     ret_object(T p, S v) {
> > -      if (p)
> > +      if (p) {
> >           *p = v;
> > +         v->retain();
> > +      }
> >        else
> >           v->release();
> >     }
> 

> diff --git a/src/gallium/state_trackers/clover/core/base.hpp b/src/gallium/state_trackers/clover/core/base.hpp
> index 19053f3..2a2e26a 100644
> --- a/src/gallium/state_trackers/clover/core/base.hpp
> +++ b/src/gallium/state_trackers/clover/core/base.hpp
> @@ -88,8 +88,7 @@ namespace clover {
>           reset(q);
>        }
>  
> -      template<typename S>
> -      ref_ptr(const ref_ptr<S> &ref) : p(NULL) {
> +      ref_ptr(const ref_ptr<T> &ref) : p(NULL) {
>           reset(ref.p);
>        }
>  




> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev