[webkit-dev] Tightening up smart pointer usage rules

[Darin Adler]

Hi folks.

I’d like to use our smart pointers more consistently to decrease the chances of memory leaks or other similar problems.

My proposal is that we have a rule that all calls to "new" are immediately followed by putting the object into a smart pointer. The main types of smart pointer that matter for these purposes are:

    RefPtr
    OwnPtr
    OwnArrayPtr

Today, we put a new reference counted object into a RefPtr by calling adoptRef. The code looks something like this:

    PassRefPtr<SharedObject> createSharedObject()
    {
        return adoptRef(new SharedObject);
    }

I propose we do the same thing with single ownership objects. It would look like this:

    PassOwnPtr<SingleOwnerObject> createSingleOwnerObject()
    {
        return adoptPtr(new SingleOwnerObject);
    }

Then it would be a programming mistake to call new without immediately calling adoptPtr or adoptRef. As part of this effort I plan to do the following:

    1) Add a debugging mode where we assert at runtime if we ref, deref, or destroy a RefCounted object before doing adoptRef. Tracked in <https://bugs.webkit.org/show_bug.cgi?id=27672>.

    2) Add an adoptPtr function that returns a PassOwnPtr, and a releasePtr function that returns a raw pointer to the PassOwnPtr class.

    3) Add a PassOwnArrayPtr with similar adoptArrayPtr and releaseArrayPtr functions.

    4) Add a strict mode to PassOwnPtr and OwnPtr which removes OwnPtr::set entirely and removes the ability to construct a PassOwnPtr or OwnPtr from a raw pointer directly, making it a compile time error to forget to use adoptPtr.

    5) Once everything compiles with the strict mode, make the strict mode the only mode.

    6) Add validator rules that make invocation of the "new" operator legal only inside adoptRef and adoptPtr function calls.

Code that used to say this:

    OwnPtr<OtherObject> m_otherObject;
    ...
    m_otherObject.set(new OtherObject);

Will now say this:

    OwnPtr<OtherObject> m_otherObject;
    ...
    m_otherObject = adoptPtr(new OtherObject);

And one thing that’s cool about that is it is quite natural for this to become:

    OwnPtr<OtherObject> m_otherObject;
    ...
    m_otherObject = OtherObject::create();

I thought I’d mention this to everyone on the list before getting doing significantly more work on this. I think it’s going to work well. Any questions or comments?

    -- Darin