Naked new considered harmful
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. Adam
Thanks, Adam. This project will be worthwhile. I think we may want to write a document about this that goes alongside <http://webkit.org/coding/RefPtr.html> because I’m sure there will be frequently-asked questions. On Aug 24, 2010, at 10:46 AM, Adam Barth wrote:
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.
This means that we might have code that looks like this: static NiftyNonRefCountedObject* singleNeverToBeDeletedInstance = adoptPtr(new NiftyNonRefCountedObject).leakPtr(); Then later, we could refactor it to look like this: static NiftyNonRefCountedObject* singleNeverToBeDeletedInstance = NiftyNonRefCountedObject::create().leakPtr(); These changes will make leaks and double deletion mistakes less likely. -- Darin
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...) Best regards, -Stephan
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus <superstippi@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. Adam
Am 25.08.2010 18:35, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus<superstippi@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. Best regards, -Stephan
On Wed, Aug 25, 2010 at 2:31 PM, Stephan Assmus <superstippi@gmx.de> wrote:
Am 25.08.2010 18:35, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus<superstippi@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
On Wed, Aug 25, 2010 at 2:34 PM, Adam Barth <abarth@webkit.org> wrote:
On Wed, Aug 25, 2010 at 2:31 PM, Stephan Assmus <superstippi@gmx.de> wrote:
Am 25.08.2010 18:35, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus<superstippi@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.
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
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
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 <marchywka@hotmail.com> 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 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
-- Sriram Neelakandan Author - Embedded Linux System Design And Development (http://tinyurl.com/2doosu)
----------------------------------------
Date: Fri, 27 Aug 2010 11:07:02 +0530 Subject: Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful] From: sriram.neelakandan@gmail.com To: marchywka@hotmail.com CC: jamesr@google.com; abarth@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 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
-- Sriram Neelakandan Author - Embedded Linux System Design And Development (http://tinyurl.com/2doosu)
Am 25.08.2010 23:34, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 2:31 PM, Stephan Assmus<superstippi@gmx.de> wrote:
Am 25.08.2010 18:35, schrieb Adam Barth:
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus<superstippi@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
On Aug 25, 2010, at 2:46 PM, Stephan Assmus wrote:
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.
If we write code to handle allocation failures at each point where we allocate memory, one thing we’d get right away is untested code paths. Before we started adding them we’d need a really clear idea of exactly what we’d gain from this along with a testing strategy. At the moment I am not aware of specific bug reports or problems that would be best solved by adding this to the project, and it’s not clear if it would do any good to add allocation failure handling in some new code without having it in the rest of the code. It’s not straightforward to handle allocation failure in constructors without exceptions, so we might have to reverse WebKit’s current policy on using C++ exceptions or make some other major design changes. For these reasons, among others, I don’t think this is a worthwhile project for WebKit. -- Darin
participants (6)
-
Adam Barth
-
Darin Adler
-
James Robinson
-
Mike Marchywka
-
Sriram Neelakandan
-
Stephan Assmus