Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4e37ddff authored by Mathias Agopian's avatar Mathias Agopian
Browse files

Fix a crasher with RefBase debugging and vectors of wp<> 

background:
we have some code to fix-up the IDs of references when
using RefBase's DEBUG_REFS when those refs are managed by
arrays wp<> or sp<> (this is because wp<> / sp<> don't have
a trivial ctor when DEBUG_REFS is enabled, and Vector
treats them as trivial for obvious performance reasons)

this is complicated by the fact that we don't want to have
to recompile everything when enabling DEBUG_REFs (i.e.: the
Vector code cannot know wheter it's enabled or not for its
template stuff).

problem:
there was a bug in the fix-up code for wp<> which was trying
to access the weakref_impl from the RefBase* however, this was
moronic since RefBase could have been destroyed if there wasn't
any more strong refs -- and this happned. Instead we need to get
the weakref_impl directly from the wp<>

Change-Id: Ie16e334204205fdbff142acb9faff8479a78450b
parent ca987c87

include/utils/RefBase.h

+56 −39
Original line number Original line Diff line number Diff line
@@ -52,12 +52,16 @@ inline bool operator _op_ (const U* o) const { \ 
}

}





// ---------------------------------------------------------------------------

// ---------------------------------------------------------------------------

class ReferenceMover;



class ReferenceConverterBase {

class ReferenceRenamer {

protected:

    // destructor is purposedly not virtual so we avoid code overhead from

    // subclasses; we have to make it protected to guarantee that it

    // cannot be called from this base class (and to make strict compilers

    // happy).

    ~ReferenceRenamer() { }

public:

public:

    virtual size_t getReferenceTypeSize() const = 0;

    virtual void operator()(size_t i) const = 0;

    virtual void* getReferenceBase(void const*) const = 0;

    inline virtual ~ReferenceConverterBase() { }

};

};





// ---------------------------------------------------------------------------

// ---------------------------------------------------------------------------
@@ -143,11 +147,6 @@ protected: 
    virtual bool            onIncStrongAttempted(uint32_t flags, const void* id);

    virtual bool            onIncStrongAttempted(uint32_t flags, const void* id);

    virtual void            onLastWeakRef(const void* id);

    virtual void            onLastWeakRef(const void* id);





private:

    friend class ReferenceMover;

    static void moveReferences(void* d, void const* s, size_t n,

            const ReferenceConverterBase& caster);



private:

private:

    friend class weakref_type;

    friend class weakref_type;

    class weakref_impl;

    class weakref_impl;
@@ -155,6 +154,17 @@ private: 
                            RefBase(const RefBase& o);

                            RefBase(const RefBase& o);

            RefBase&        operator=(const RefBase& o);

            RefBase&        operator=(const RefBase& o);





private:

    friend class ReferenceMover;



    static void renameRefs(size_t n, const ReferenceRenamer& renamer);



    static void renameRefId(weakref_type* ref,

            const void* old_id, const void* new_id);



    static void renameRefId(RefBase* ref,

            const void* old_id, const void* new_id);



        weakref_impl* const mRefs;

        weakref_impl* const mRefs;

};

};
@@ -185,8 +195,9 @@ protected: 




private:

private:

    friend class ReferenceMover;

    friend class ReferenceMover;

    inline static void moveReferences(void* d, void const* s, size_t n,

    inline static void renameRefs(size_t n, const ReferenceRenamer& renamer) { }

            const ReferenceConverterBase& caster) { }

    inline static void renameRefId(T* ref,

            const void* old_id, const void* new_id) { }





private:

private:

    mutable volatile int32_t mCount;

    mutable volatile int32_t mCount;
@@ -455,42 +466,48 @@ inline TextOutput& operator<<(TextOutput& to, const wp<T>& val) 




// this class just serves as a namespace so TYPE::moveReferences can stay

// this class just serves as a namespace so TYPE::moveReferences can stay

// private.

// private.



class ReferenceMover {

class ReferenceMover {

    // StrongReferenceCast and WeakReferenceCast do the impedance matching

public:

    // between the generic (void*) implementation in Refbase and the strongly typed

    // it would be nice if we could make sure no extra code is generated

    // template specializations below.

    // for sp<TYPE> or wp<TYPE> when TYPE is a descendant of RefBase:

    // Using a sp<RefBase> override doesn't work; it's a bit like we wanted

    // a template<typename TYPE inherits RefBase> template...





    template <typename TYPE>

    template<typename TYPE> static inline

    struct StrongReferenceCast : public ReferenceConverterBase {

    void move_references(sp<TYPE>* d, sp<TYPE> const* s, size_t n) {

        virtual size_t getReferenceTypeSize() const { return sizeof( sp<TYPE> ); }

        virtual void* getReferenceBase(void const* p) const {

            sp<TYPE> const* sptr(reinterpret_cast<sp<TYPE> const*>(p));

            return static_cast<typename TYPE::basetype *>(sptr->get());

        }

    };





    template <typename TYPE>

        class Renamer : public ReferenceRenamer {

    struct WeakReferenceCast : public ReferenceConverterBase {

            sp<TYPE>* d;

        virtual size_t getReferenceTypeSize() const { return sizeof( wp<TYPE> ); }

            sp<TYPE> const* s;

        virtual void* getReferenceBase(void const* p) const {

            virtual void operator()(size_t i) const {

            wp<TYPE> const* sptr(reinterpret_cast<wp<TYPE> const*>(p));

                // The id are known to be the sp<>'s this pointer

            return static_cast<typename TYPE::basetype *>(sptr->unsafe_get());

                TYPE::renameRefId(d[i].get(), &s[i], &d[i]);

            }

            }

        public:

            Renamer(sp<TYPE>* d, sp<TYPE> const* s) : s(s), d(d) { }

        };

        };





public:

    template<typename TYPE> static inline

    void move_references(sp<TYPE>* d, sp<TYPE> const* s, size_t n) {

        memmove(d, s, n*sizeof(sp<TYPE>));

        memmove(d, s, n*sizeof(sp<TYPE>));

        StrongReferenceCast<TYPE> caster;

        TYPE::renameRefs(n, Renamer(d, s));

        TYPE::moveReferences(d, s, n, caster);

    }

    }





    template<typename TYPE> static inline

    template<typename TYPE> static inline

    void move_references(wp<TYPE>* d, wp<TYPE> const* s, size_t n) {

    void move_references(wp<TYPE>* d, wp<TYPE> const* s, size_t n) {



        class Renamer : public ReferenceRenamer {

            wp<TYPE>* d;

            wp<TYPE> const* s;

            virtual void operator()(size_t i) const {

                // The id are known to be the wp<>'s this pointer

                TYPE::renameRefId(d[i].get_refs(), &s[i], &d[i]);

            }

        public:

            Renamer(wp<TYPE>* d, wp<TYPE> const* s) : s(s), d(d) { }

        };



        memmove(d, s, n*sizeof(wp<TYPE>));

        memmove(d, s, n*sizeof(wp<TYPE>));

        WeakReferenceCast<TYPE> caster;

        TYPE::renameRefs(n, Renamer(d, s));

        TYPE::moveReferences(d, s, n, caster);

    }

    }

};

};

libs/utils/RefBase.cpp

+15 −9
Original line number Original line Diff line number Diff line
@@ -631,21 +631,27 @@ void RefBase::onLastWeakRef(const void* /*id*/) 




// ---------------------------------------------------------------------------

// ---------------------------------------------------------------------------





void RefBase::moveReferences(void* dst, void const* src, size_t n,

void RefBase::renameRefs(size_t n, const ReferenceRenamer& renamer) {

        const ReferenceConverterBase& caster)

{

#if DEBUG_REFS

#if DEBUG_REFS

    const size_t itemSize = caster.getReferenceTypeSize();

    for (size_t i=0 ; i<n ; i++) {

    for (size_t i=0 ; i<n ; i++) {

        void*       d = reinterpret_cast<void      *>(intptr_t(dst) + i*itemSize);

        renamer(i);

        void const* s = reinterpret_cast<void const*>(intptr_t(src) + i*itemSize);

        RefBase* ref(reinterpret_cast<RefBase*>(caster.getReferenceBase(d)));

        ref->mRefs->renameStrongRefId(s, d);

        ref->mRefs->renameWeakRefId(s, d);

    }

    }

#endif

#endif

}

}





void RefBase::renameRefId(weakref_type* ref,

        const void* old_id, const void* new_id) {

    weakref_impl* const impl = static_cast<weakref_impl*>(ref);

    impl->renameStrongRefId(old_id, new_id);

    impl->renameWeakRefId(old_id, new_id);

}



void RefBase::renameRefId(RefBase* ref,

        const void* old_id, const void* new_id) {

    ref->mRefs->renameStrongRefId(old_id, new_id);

    ref->mRefs->renameWeakRefId(old_id, new_id);

}



// ---------------------------------------------------------------------------

// ---------------------------------------------------------------------------





TextOutput& printStrongPointer(TextOutput& to, const void* val)

TextOutput& printStrongPointer(TextOutput& to, const void* val)