Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
> Date: Fri, 27 Aug 2010 11:07:02 +0530
> Subject: Re: [webkit-dev] Checking allocation failures [was: Re: Naked new
> considered harmful]
> From: sriram.neelakan...@gmail.com
> To: marchy...@hotmail.com
> CC: jam...@google.com; aba...@webkit.org; webkit-dev@lists.webkit.org
>
> Hi all,
>
> I am still a newbie in this ocenaous wekbit ... here are some
> thoughts from me.. please correct me if i have misunderstood some
> thing;
>
> Talking on the same lines of Resource allocations.. I have seen
> already snips of code that handle Large Resource allocations
> gracefully. For instance i think the ImageReosource has a check on
> maximum decoded image size..But that just applies to a single image
> being decoded;
That is just an example I use because I've seen it happen- sure the
image can be too big but the data can be corrupt too.
> What webkit may need is something like maximum amount of memory
> available for all Image Resources in a page.. and when the limit is
> hit inform the Client/embedder of the same.
> So basically in this case the page can get gracefully degraded and the
> user can be informed of the same; I think some other stuff like CSS
> can also be gracefully handled
I guess there are two issues- being able to kill the thing least relevant
to the user when you encounter resource limitations and optimizing
the use of resources prior to that. One issue with the latter is often memory
coherence- no one wants to here my stories about the disk light coming
on everytime I try to fill out a web form- and this goes along with
things like Planner or Strategy or whatever you want to call them.
If everytime you were about to do something "significant" you may an estimate
of the memory or IO or CPU cycles to accomplish some part of that task,
first you could maybe pick among implementations suited to the immediate
situation and then you could allocate memory in a single heap for that task.
I guess my only point is that probably many exceptions due to alloc failure
may be predictable with effort that is not a distraction from larger issues like
optimiztion.
As a browser user there are many times when I'd simply like to download text
instead of images, certainly turn off Flash as when I'm on a modem. It may
be possible to set criteria however rather than just on/off(" turn off images
if bw hits 56kbps unless I click on the substitute text"). I guess you could
consider a feature like this to be "picking" among rendering things that render
images as their alt text or not.
>
> But Now lets say there is a cap for maximum JS memory and we run out
> of it some reason; then it would be safe to inform the user and unload
> the page itself;
This always bothers me- hard limits need to be set somehow. And ok sure
there is probably a fixed amount of memory on the platform that
gives you some numbers to work from but still trying to limit certain
things to fixed amounts seems to create arbitrary things that may be had
to pick well.
>
> I have seen lot of cache storage code that is already checking for
> resource object sizes (including decoded sizes).. I think this may be
> a worthy to try
>
> On 8/26/10, Mike Marchywka wrote:
> >
> >
> >> >>> 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
> >> &g
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
Hi all, I am still a newbie in this ocenaous wekbit ... here are some thoughts from me.. please correct me if i have misunderstood some thing; Talking on the same lines of Resource allocations.. I have seen already snips of code that handle Large Resource allocations gracefully. For instance i think the ImageReosource has a check on maximum decoded image size..But that just applies to a single image being decoded; What webkit may need is something like maximum amount of memory available for all Image Resources in a page.. and when the limit is hit inform the Client/embedder of the same. So basically in this case the page can get gracefully degraded and the user can be informed of the same; I think some other stuff like CSS can also be gracefully handled But Now lets say there is a cap for maximum JS memory and we run out of it some reason; then it would be safe to inform the user and unload the page itself; I have seen lot of cache storage code that is already checking for resource object sizes (including decoded sizes).. I think this may be a worthy to try On 8/26/10, Mike Marchywka wrote: > > >> >>> 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. >> >> We do have a tryFastMalloc() path that returns NULL on allocation >> failure instead of crashing. It's used in some pieces of code that are >> carefully written to handle an allocation failure gracefully. >> However, this is rare. >> >> In general it's very difficult to recover from an allocation failure in >> an arbitrary piece of code with an arbitrary callstack. >> >> - James >> > > ( hotmail is still having all kinds of problems, doesn't seem to like text > or a modem connection, sorry for eidting slop ). > > I guess this relates to many earlier comments including that issue with the > linker reporting a memory problem LOL. I'm just thinking out loud since no > one on thread > seems to have a lot of conviction so far. > You could probably make a list of reasons why an allocation could fail: > 1) the app is shutting down or OS is shutting down, > 2) what you are trying todo is too big for the platform, > 3) someone else has the resources you need for a tolerable or intolerable > time, > 4) you have bad data and calculated a size based on absurd numbers. > 5) Algorithm failure, you have reasonable data but either code or algorirthm > has a bug or impractical way of handling the data. > > Hopefully, you aren't just allocating memory on the spur of the moment," oh > look I need more memory how about that," and could maybe even consider > planning ahead. Presumably this helps validate your data, or at least sanity > check it, maybe if you have 1<<30( hotmail somtimes tries to interpret gt, I > mean of course 2^30 LOL if the > shift operator got mangled ) > and 1<<30 they really are not the width and height of an image or one > you could practically show on the platform. > > I guess first it would help to make sure the platform API's give you > abilities to > determine what makes sense for the platform and some introspective API gave > you some ability to understand who has what and why. Then more use of > strategy > type classes or planners could let you estimate memory needs and even > allocate > things together to improve coherence. > > It can be hard to clean up a partially created thing like a tab but if you > group the memory allocations at a logical unit of some kind it would be a > lot > easier to just give up without trying. It is probably possible to warn the > user > too, " you have too much open" just as some browsers give the " a script is > causing > your computer to run slowly" message. > > > While this is a bit grandiose I'm just trying to focus conversation and > r
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
> >>> 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. > > We do have a tryFastMalloc() path that returns NULL on allocation > failure instead of crashing. It's used in some pieces of code that are > carefully written to handle an allocation failure gracefully. > However, this is rare. > > In general it's very difficult to recover from an allocation failure in > an arbitrary piece of code with an arbitrary callstack. > > - James > ( hotmail is still having all kinds of problems, doesn't seem to like text or a modem connection, sorry for eidting slop ). I guess this relates to many earlier comments including that issue with the linker reporting a memory problem LOL. I'm just thinking out loud since no one on thread seems to have a lot of conviction so far. You could probably make a list of reasons why an allocation could fail: 1) the app is shutting down or OS is shutting down, 2) what you are trying todo is too big for the platform, 3) someone else has the resources you need for a tolerable or intolerable time, 4) you have bad data and calculated a size based on absurd numbers. 5) Algorithm failure, you have reasonable data but either code or algorirthm has a bug or impractical way of handling the data. Hopefully, you aren't just allocating memory on the spur of the moment," oh look I need more memory how about that," and could maybe even consider planning ahead. Presumably this helps validate your data, or at least sanity check it, maybe if you have 1<<30( hotmail somtimes tries to interpret gt, I mean of course 2^30 LOL if the shift operator got mangled ) and 1<<30 they really are not the width and height of an image or one you could practically show on the platform. I guess first it would help to make sure the platform API's give you abilities to determine what makes sense for the platform and some introspective API gave you some ability to understand who has what and why. Then more use of strategy type classes or planners could let you estimate memory needs and even allocate things together to improve coherence. It can be hard to clean up a partially created thing like a tab but if you group the memory allocations at a logical unit of some kind it would be a lot easier to just give up without trying. It is probably possible to warn the user too, " you have too much open" just as some browsers give the " a script is causing your computer to run slowly" message. While this is a bit grandiose I'm just trying to focus conversation and relate it to my fixation on memory coherence and resource usage. > > Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
Am 25.08.2010 23:34, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 2:31 PM, Stephan Assmus wrote:
Am 25.08.2010 18:35, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus
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 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
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
On Wed, Aug 25, 2010 at 2:34 PM, Adam Barth wrote:
> On Wed, Aug 25, 2010 at 2:31 PM, Stephan Assmus
> wrote:
> > Am 25.08.2010 18:35, schrieb Adam Barth:
> >> On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus
> >> 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 PassOwnPtrcreate(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.
>
We do have a tryFastMalloc() path that returns NULL on allocation failure
instead of crashing. It's used in some pieces of code that are carefully
written to handle an allocation failure gracefully. However, this is rare.
In general it's very difficult to recover from an allocation failure in an
arbitrary piece of code with an arbitrary callstack.
- James
> Adam
> ___
> 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] Checking allocation failures [was: Re: Naked new considered harmful]
On Wed, Aug 25, 2010 at 2:31 PM, Stephan Assmus wrote:
> Am 25.08.2010 18:35, schrieb Adam Barth:
>> On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus
>> 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 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
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
Am 25.08.2010 18:35, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus 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 PassOwnPtrcreate(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.
Best regards,
-Stephan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus 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 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.
Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
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 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...)
Best regards,
-Stephan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
