Am 25.08.2010 23:34, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 2:31 PM, Stephan Assmus<supersti...@gmx.de> wrote:
Am 25.08.2010 18:35, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus<supersti...@gmx.de>
wrote:
Am 24.08.2010 19:46, schrieb Adam Barth:
One thing Darin and I discussed at WWDC (yes, this email has been a
long time coming) is better programming patterns to prevent memory
leaks. As I'm sure you know, whenever you allocate a RefCounted
object, you need to call adoptRef to prevent a memory leak by adopting
the object's initial reference:
adoptRef(new FancyRefCountedObject())
We now have an ASSERT that enforces this programming pattern, so we
should be able to catch errors pretty easily. Recently, Darin also
introduced an analogous function for OwnPtr:
adoptPtr(new NiftyNonRefCountedObject())
What adoptPtr does is shove the newly allocated object into a
PassOwnPtr, which together with OwnPtr, makes sure we don't leak the
object. By using one of these two functions every time we call new,
it's easy to see that we don't have any memory leaks. In the cases
where we have an intentional memory leak (e.g., for a static), please
use the leakPtr() member function to document the leak.
In writing new code, please avoid using "naked" calls to new. If
you're making an object that you expect to be heap-allocated, please
add a "create" method, similar to the create method we use for
RefCounted objects:
static PassOwnPtr<NiftyNonRefCountedObject> create(ParamClass* param)
{
return adoptPtr(new NiftyNonRefCountedObject(param));
}
You should also make the constructor non-public so folks are forced to
call the create method. (Of course, stack-allocated objects should
have public constructors.)
I'm going through WebCore and removing as many naked news and I can.
At some point, we'll add a stylebot rule to flag violations for new
code. If you'd like to help out, pick a directory and change all the
calls to new to use adoptRef or adoptPtr, as appropriate.
this reminds me that I've always been wondering about checks for
allocation
failure in WebCore (versus concerns about leaks). A plain call to new may
throw an std::bad_alloc exception. If this is not considered, it may
leave
objects in an invalid state, even when using objects such as RefPtr to
wrap
arround allocations. I don't remember any specific place in the WebCore
code
where it would be a problem, I just don't remember seeing any code that
deals with allocation failures. Is this a design choice somehow? If so,
is
there some online documentation/discussion about it? (Tried to google it
but
found nothing...)
Failed allocations in WebKit call CRASH(). This prevents us from
ending up in an inconsistent state.
Ok, but WebKit is used on devices where allocation failure is somewhat of a
realistic possibility, no? Wouldn't it be desirable to handle such a
situation gracefully? (I.e. display of an error message when trying to open
one more tab rather than crashing the entire browser, for example.)
Hopefully I am not missing something obvious.
Dunno. The crash-on-allocation failure assumption is baked into lots
of lines of code.
I just thought that if my observations are correct, and on the subject
of advertising a certain way to write code (with regards to your initial
email), perhaps new code (and eventually old code) should also follow a
guideline that allows to handle allocation failures gracefully. For
example, if no allocations are to be done in constructors, but rather
within a dedicated init() method, objects remain always valid, even if
init() throws half-way through, and they could be deallocated gracefully.
Best regards,
-Stephan
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
