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. Adam _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev