Re: [webkit-dev] Tightening up smart pointer usage rules
btw, looking at Darin's earlier note about releaseRef. I think it would be consistent to rename releaseRef to be leakRef. On Tue, Jun 29, 2010 at 11:14 AM, Geoffrey Garen wrote: > > I like leakPtr. > > > > Personally, releasePtr() is so close to release() (which is safe) that I > could easily overlook it in a code review and not realize that it hands off > a raw pointer with a ref count on it. > > > > dave > > +1 > > Anders once argued against the name leakPtr, "Well, then you might as well > rename 'malloc' to 'leak'." > > The more I think about it, though, the more I think that "malloc" really > should be named "leak", at least from the WebKit project's perspective. > > Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Tightening up smart pointer usage rules
> I like leakPtr. > > Personally, releasePtr() is so close to release() (which is safe) that I > could easily overlook it in a code review and not realize that it hands off a > raw pointer with a ref count on it. > > dave +1 Anders once argued against the name leakPtr, "Well, then you might as well rename 'malloc' to 'leak'." The more I think about it, though, the more I think that "malloc" really should be named "leak", at least from the WebKit project's perspective. Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Tightening up smart pointer usage rules
I like leakPtr. Personally, releasePtr() is so close to release() (which is safe) that I could easily overlook it in a code review and not realize that it hands off a raw pointer with a ref count on it. dave On Tue, Jun 29, 2010 at 10:40 AM, Darin Adler wrote: > On Jun 28, 2010, at 7:08 PM, Maciej Stachowiak wrote: > > > - Yes, we should get rid of auto_ptr. > > - I can imagine a leakPtr function being more self-documenting than > adoptPtr(...).releasePtr() when it appears in code, but on the other hand > the greater convenience may lead to using it carelessly. On the third hand, > it will definitely stand out if it appears in a patch. > > The main benefit of adoptPtr(...).releasePtr() is that the adoptPtr can > stay with the new when refactoring, and the releasePtr can stay with the > intentional leak. > > Any call site using releasePtr needs to be examined carefully to see it > does not leak. That’s different from functions like release that do not pose > that sort of risk, but similar to releaseRef, which does. > > If we think the word "leak" is key to clarity here, renaming releasePtr to > leakPtr would be one option. I don’t think we want a single function > combining the adoptPtr and releasePtr behavior. > >-- Darin > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Tightening up smart pointer usage rules
On Jun 28, 2010, at 7:08 PM, Maciej Stachowiak wrote: > - Yes, we should get rid of auto_ptr. > - I can imagine a leakPtr function being more self-documenting than > adoptPtr(...).releasePtr() when it appears in code, but on the other hand the > greater convenience may lead to using it carelessly. On the third hand, it > will definitely stand out if it appears in a patch. The main benefit of adoptPtr(...).releasePtr() is that the adoptPtr can stay with the new when refactoring, and the releasePtr can stay with the intentional leak. Any call site using releasePtr needs to be examined carefully to see it does not leak. That’s different from functions like release that do not pose that sort of risk, but similar to releaseRef, which does. If we think the word "leak" is key to clarity here, renaming releasePtr to leakPtr would be one option. I don’t think we want a single function combining the adoptPtr and releasePtr behavior. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Tightening up smart pointer usage rules
I like this idea. Addressing some of the side comments:
- Yes, we should get rid of auto_ptr.
- I can imagine a leakPtr function being more self-documenting than
adoptPtr(...).releasePtr() when it appears in code, but on the other hand the
greater convenience may lead to using it carelessly. On the third hand, it will
definitely stand out if it appears in a patch.
Regards,
Maciej
On Jun 28, 2010, at 1:39 PM, Darin Adler wrote:
> 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 createSharedObject()
>{
>return adoptRef(new SharedObject);
>}
>
> I propose we do the same thing with single ownership objects. It would look
> like this:
>
>PassOwnPtr 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
> .
>
>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 m_otherObject;
>...
>m_otherObject.set(new OtherObject);
>
> Will now say this:
>
>OwnPtr 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 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
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Tightening up smart pointer usage rules
On 06/29/2010 04:39 AM, Darin Adler wrote: > 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. I like that, it would have found/avoided a memleak for SimpleFontData we had in Gtk+ and spread to more platforms.. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Tightening up smart pointer usage rules
On Jun 28, 2010, at 2:55 PM, Antti Koivisto wrote: > Is the plan also to banish the std::auto_ptr? It seems pointless and > confusing to allow both the OwnPtr and the auto_ptr. Yes, that would be my preference. PassOwnPtr is intended as a replacement for std::auto_ptr. Before PassOwnPtr existed, we used auto_ptr, but now we should replace any use of auto_ptr with PassOwnPtr and OwnPtr. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Tightening up smart pointer usage rules
On Jun 28, 2010, at 2:14 PM, Adam Barth wrote: > Do we need an exception for statics that we intend to leak at shutdown? Maybe > leakPtr(new Foo)? Maybe. My first thought for such things is that I’d prefer to write them as: adoptPtr(new Foo).releasePtr() At first that may look strange, but it has a huge benefit. It can be refactored to: Foo::create().releasePtr() That’s the main reason I don’t suggest having a leakPtr or dontAdoptPtr function. I have thought that leakPtr might be a good alternate name for releasePtr. In fact I have proposed renaming releaseRef to leakRef more than once. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Tightening up smart pointer usage rules
Is the plan also to banish the std::auto_ptr? It seems pointless and
confusing to allow both the OwnPtr and the auto_ptr.
antti (who wants PwnPtr too)
On Mon, Jun 28, 2010 at 11:39 PM, Darin Adler wrote:
> 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 createSharedObject()
> {
> return adoptRef(new SharedObject);
> }
>
> I propose we do the same thing with single ownership objects. It would look
> like this:
>
> PassOwnPtr 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
> .
>
> 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 m_otherObject;
> ...
> m_otherObject.set(new OtherObject);
>
> Will now say this:
>
> OwnPtr 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 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
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Tightening up smart pointer usage rules
Sounds great. Let me know how I can help.
Do we need an exception for statics that we intend to leak at
shutdown? Maybe leakPtr(new Foo)?
Adam
On Mon, Jun 28, 2010 at 1:39 PM, Darin Adler wrote:
> 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 createSharedObject()
> {
> return adoptRef(new SharedObject);
> }
>
> I propose we do the same thing with single ownership objects. It would look
> like this:
>
> PassOwnPtr 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
> .
>
> 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 m_otherObject;
> ...
> m_otherObject.set(new OtherObject);
>
> Will now say this:
>
> OwnPtr 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 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
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Tightening up smart pointer usage rules
On Mon, Jun 28, 2010 at 1:43 PM, Kenneth Christiansen wrote: >> 6) Add validator rules that make invocation of the "new" operator legal >> only inside adoptRef and adoptPtr function calls. > > That is probably going to be a problem for Qt code on the WebKit API > side. If we disable this rule for WebKitTools and WebKit I think we > should be OK We already have similar exceptions in the validator for code in the embedder's style. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Tightening up smart pointer usage rules
> 6) Add validator rules that make invocation of the "new" operator legal > only inside adoptRef and adoptPtr function calls. That is probably going to be a problem for Qt code on the WebKit API side. If we disable this rule for WebKitTools and WebKit I think we should be OK Kenneth ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Tightening up smart pointer usage rules
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 createSharedObject()
{
return adoptRef(new SharedObject);
}
I propose we do the same thing with single ownership objects. It would look
like this:
PassOwnPtr 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
.
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 m_otherObject;
...
m_otherObject.set(new OtherObject);
Will now say this:
OwnPtr 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 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
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
