Out of memory exception - graphics
-
Hi Luc, TheComputerMan wrote: Catch ex As Exception Return Nothing End Try This code is stupid: whatever goes wrong you choose to just ignore it; and then wonder what is going wrong and post a question here? 1. A completely erroneous assumption. First of all you assume that I do not know what the error is which I do, second you assume that it is ignored - it is not as the graphic = nothing triggers code which places the problem filename in List(of String) which is then displayed to the user in a list box. The post was NOT because I do not know what is going on. Credit me with a bit more common sense that that. 2. Thread.Sleep was just experimental to see if it would improve things - and I said that in the original post. 3. Is it useless??? Are you sure about that. I have experience of it kicking off the garbage collector in another app which was not kicking off after a dispose was performed and was causing similar problems. 4. You have a point there. This would only be a problem however if the error occurred, but would always be executed while it does not so since the error occurs and then the object does not get released this is not the cause of the problem. It seems to me that you basically have made one good point and the rest is just unconstructive demeaning comment. This still leaves the problem however!
I looked at the code you have shown and gave you my ideas, as you requested. If you don't like them, that is too bad. If you know better, good for you. But then, who is having a problem? :|
Luc Pattyn
Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.
Local announcement (Antwerp region): Lange Wapper? Neen!
-
Hi Luc, TheComputerMan wrote: Catch ex As Exception Return Nothing End Try This code is stupid: whatever goes wrong you choose to just ignore it; and then wonder what is going wrong and post a question here? 1. A completely erroneous assumption. First of all you assume that I do not know what the error is which I do, second you assume that it is ignored - it is not as the graphic = nothing triggers code which places the problem filename in List(of String) which is then displayed to the user in a list box. The post was NOT because I do not know what is going on. Credit me with a bit more common sense that that. 2. Thread.Sleep was just experimental to see if it would improve things - and I said that in the original post. 3. Is it useless??? Are you sure about that. I have experience of it kicking off the garbage collector in another app which was not kicking off after a dispose was performed and was causing similar problems. 4. You have a point there. This would only be a problem however if the error occurred, but would always be executed while it does not so since the error occurs and then the object does not get released this is not the cause of the problem. It seems to me that you basically have made one good point and the rest is just unconstructive demeaning comment. This still leaves the problem however!
Though I agree Luc could have been a little nicer, he's right on all four points. 1) If you know what the exception is, and are intentionally ignoring it (Not ideal, but sometimes unavoidable), you should at least catch the specific exception being triggered... For example, "Catch ex as FileNotFoundException", so you won't miss an unexpected one. 2) As he said, sleeping the thread won't help, but you realized that. 3) Yes, setting a local variable to nothing is meaningless, since it will go out of scope as soon as the method ends. This was needed back in VB6 when you didn't have a Dispose() method, but no longer. 4) Definitely put all disposes in Finally blocks, otherwise they'll be skipped if an exception occurs. If you want to keep things simple, define both disposables at the beginning of the method (Initialized to null), and only try to dispose if they're non-null. That way you don't need an extra try loop just for the graphics object. Other suggestions: 5) Don't use "new IntPtr()"... Use IntPtr.Zero 6) From the MSDN article for GetThumbnailImage, in reference to the callback parameter: "In GDI+ version 1.0, the delegate is not used. Even so, you must create a delegate and pass a reference to that delegate in this parameter"... As we don't know how this is implemented (By Microsoft), this may or may not be the cause. If none of these solve your problem, it may be elsewhere in your program... Just because this method does the heavy lifting, doesn't necessarily mean it's the only one that can leak.
Proud to have finally moved to the A-Ark. Which one are you in? Developer, Author (Guardians of Xen)
-
Hi Luc, TheComputerMan wrote: Catch ex As Exception Return Nothing End Try This code is stupid: whatever goes wrong you choose to just ignore it; and then wonder what is going wrong and post a question here? 1. A completely erroneous assumption. First of all you assume that I do not know what the error is which I do, second you assume that it is ignored - it is not as the graphic = nothing triggers code which places the problem filename in List(of String) which is then displayed to the user in a list box. The post was NOT because I do not know what is going on. Credit me with a bit more common sense that that. 2. Thread.Sleep was just experimental to see if it would improve things - and I said that in the original post. 3. Is it useless??? Are you sure about that. I have experience of it kicking off the garbage collector in another app which was not kicking off after a dispose was performed and was causing similar problems. 4. You have a point there. This would only be a problem however if the error occurred, but would always be executed while it does not so since the error occurs and then the object does not get released this is not the cause of the problem. It seems to me that you basically have made one good point and the rest is just unconstructive demeaning comment. This still leaves the problem however!
TheComputerMan wrote:
This code is stupid: whatever goes wrong you choose to just ignore it; and then wonder what is going wrong and post a question here? 1. A completely erroneous assumption
No, it's not erroneous. You said you know what THE (assuming one) exception is, but you chose to capture ALL exceptions and eat them. Catch the ONE exception you know is going to come and handle it appropriately, log all the rest so you can see any other errors that show up.
TheComputerMan wrote:
3. Is it useless??? Are you sure about that. I have experience of it kicking off the garbage collector in another app which was not kicking off after a dispose was performed
The garbage collector doesn't wait for you app to pause. It'll pause your app for you and run when it needs to. Just throwing a sleep in doesn't kick off nor does it allow to run the GC. The GC runs when it wants to, regardless of what your app says. If you're doing this to "get the GC to run", you've got another problem in your code that this code just happens to work around. I really wouldn't call it a production quality fix.
A guide to posting questions on CodeProject[^]
Dave Kreskowiak Microsoft MVP Visual Developer - Visual Basic
2006, 2007, 2008
But no longer in 2009... -
I looked at the code you have shown and gave you my ideas, as you requested. If you don't like them, that is too bad. If you know better, good for you. But then, who is having a problem? :|
Luc Pattyn
Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.
Local announcement (Antwerp region): Lange Wapper? Neen!
Luc, my apologies but I think I did not get what you were saying, and I did not see them as ideas merely harsh criticism.
-
Though I agree Luc could have been a little nicer, he's right on all four points. 1) If you know what the exception is, and are intentionally ignoring it (Not ideal, but sometimes unavoidable), you should at least catch the specific exception being triggered... For example, "Catch ex as FileNotFoundException", so you won't miss an unexpected one. 2) As he said, sleeping the thread won't help, but you realized that. 3) Yes, setting a local variable to nothing is meaningless, since it will go out of scope as soon as the method ends. This was needed back in VB6 when you didn't have a Dispose() method, but no longer. 4) Definitely put all disposes in Finally blocks, otherwise they'll be skipped if an exception occurs. If you want to keep things simple, define both disposables at the beginning of the method (Initialized to null), and only try to dispose if they're non-null. That way you don't need an extra try loop just for the graphics object. Other suggestions: 5) Don't use "new IntPtr()"... Use IntPtr.Zero 6) From the MSDN article for GetThumbnailImage, in reference to the callback parameter: "In GDI+ version 1.0, the delegate is not used. Even so, you must create a delegate and pass a reference to that delegate in this parameter"... As we don't know how this is implemented (By Microsoft), this may or may not be the cause. If none of these solve your problem, it may be elsewhere in your program... Just because this method does the heavy lifting, doesn't necessarily mean it's the only one that can leak.
Proud to have finally moved to the A-Ark. Which one are you in? Developer, Author (Guardians of Xen)
Ian, many thanks for your response. I understand what was being said now and yes the code was not the best! :-O I have made all the changes that you and Luc suggested, but it still gives an Out of memory error after about 65 or so iterations. I have had to leave Nothing in the callback as both Microsoft's example and the other example given in MSDN do not work for me. The Microsoft one gives the message "'System.Drawing.Image.GetThumbnailImageAbort' is a delegate type and requires a single 'addressof' expression as the only argument to the constructor." ?? What does that mean. I copied the code which was in c# and converted it to VB on Developer Fusion's web page and that was the result. Using the other example I get 'Value of type 'Boolean' cannot be converted to 'System.Drawing.Image.GetThumbnailImageAbort' Nothing does not seem to cause any problems!!
-
Ian, many thanks for your response. I understand what was being said now and yes the code was not the best! :-O I have made all the changes that you and Luc suggested, but it still gives an Out of memory error after about 65 or so iterations. I have had to leave Nothing in the callback as both Microsoft's example and the other example given in MSDN do not work for me. The Microsoft one gives the message "'System.Drawing.Image.GetThumbnailImageAbort' is a delegate type and requires a single 'addressof' expression as the only argument to the constructor." ?? What does that mean. I copied the code which was in c# and converted it to VB on Developer Fusion's web page and that was the result. Using the other example I get 'Value of type 'Boolean' cannot be converted to 'System.Drawing.Image.GetThumbnailImageAbort' Nothing does not seem to cause any problems!!
No problems on the surface, but maybe a memory leak internally... Graphics calls tend to have issues like that... Basically it wants a delegate of a particular type, in this case a
Image.GetThumbnailImageAbortdelegate. As per MSDN, you need to create a dummy function, such as:Private Function ThumbnailDummyFunc() as Boolean
End Function'And in the GetThumbnailImage call, you would pass this as the argument:
new Image.GetThumbnailImageAbort(AddressOf ThumbnailDummyFunc)(Sorry if my syntax is off... Been programming in C# all day, so it's tricky to switch mental modes)
Proud to have finally moved to the A-Ark. Which one are you in? Developer, Author (Guardians of Xen)
-
Luc, my apologies but I think I did not get what you were saying, and I did not see them as ideas merely harsh criticism.
Sorry, no harsh criticism intended; however I get upset when exceptions get ignored like that. They exist to signal a problem, so don't shut them out. One more idea: Image.FromFile throws an OOM on a bad file, so I suggest you check which file fails; then try the sequence again, skipping the first half of the succeeding files. :)
Luc Pattyn
Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.
Local announcement (Antwerp region): Lange Wapper? Neen!
-
No problems on the surface, but maybe a memory leak internally... Graphics calls tend to have issues like that... Basically it wants a delegate of a particular type, in this case a
Image.GetThumbnailImageAbortdelegate. As per MSDN, you need to create a dummy function, such as:Private Function ThumbnailDummyFunc() as Boolean
End Function'And in the GetThumbnailImage call, you would pass this as the argument:
new Image.GetThumbnailImageAbort(AddressOf ThumbnailDummyFunc)(Sorry if my syntax is off... Been programming in C# all day, so it's tricky to switch mental modes)
Proud to have finally moved to the A-Ark. Which one are you in? Developer, Author (Guardians of Xen)
Hi Ian, I think I sussed out the delegate thing. I ended up with this:
Dim callback As New Image.GetThumbnailImageAbort(AddressOf ThumbnailCallback)
and
Public Function ThumbnailCallback() As Boolean Return False End Functionso my getthumbnailimage call looks like this now - but the error still occurs.
destBitmap = sourceBitmap.GetThumbnailImage(imgWidth, imgHeight, callback, IntPtr.Zero)
Looking at the list of errors there is one 'out of memory', then two 'Parameter is not valid' and then the rest are 'Out of Memory'. I don't know where this 'Parameter is not valid' is coming from so I shall try and single step the whole thing (after the first error)
-
Hi Ian, I think I sussed out the delegate thing. I ended up with this:
Dim callback As New Image.GetThumbnailImageAbort(AddressOf ThumbnailCallback)
and
Public Function ThumbnailCallback() As Boolean Return False End Functionso my getthumbnailimage call looks like this now - but the error still occurs.
destBitmap = sourceBitmap.GetThumbnailImage(imgWidth, imgHeight, callback, IntPtr.Zero)
Looking at the list of errors there is one 'out of memory', then two 'Parameter is not valid' and then the rest are 'Out of Memory'. I don't know where this 'Parameter is not valid' is coming from so I shall try and single step the whole thing (after the first error)
It is this line sourceBitmap = New Bitmap(Image.FromFile(ImagePathName)) This causes the Out Of Memory errors and the Parameter is not valid. When I get the parameter error, the file name is a valid existing file name and is a graphic. If I wait two minutes and then complete the list it works, in other words the file that seemed to cause an error, and all the subsequent files in the list, go through OK. :confused: :confused:
-
Sorry, no harsh criticism intended; however I get upset when exceptions get ignored like that. They exist to signal a problem, so don't shut them out. One more idea: Image.FromFile throws an OOM on a bad file, so I suggest you check which file fails; then try the sequence again, skipping the first half of the succeeding files. :)
Luc Pattyn
Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.
Local announcement (Antwerp region): Lange Wapper? Neen!
Thanks Luc, you may have a good point there. I will skip the first files and see if I get the error.
-
Thanks Luc, you may have a good point there. I will skip the first files and see if I get the error.
Nope, it went straight through and did the lot (40+ files) no problem. It definitely seems to stil at 66 files.
-
Nope, it went straight through and did the lot (40+ files) no problem. It definitely seems to stil at 66 files.
Strange but true Luc, you sort of hit the nail! I did this:
sr = New StreamReader(ImagePathName) 'sourceBitmap = New Bitmap(Image.FromFile(ImagePathName)) sourceBitmap = New Bitmap(Image.FromStream(sr.BaseStream))Reading it into a stream first works - no memory errors! Thanks for you assistance.(and tutoring :) )
-
It is this line sourceBitmap = New Bitmap(Image.FromFile(ImagePathName)) This causes the Out Of Memory errors and the Parameter is not valid. When I get the parameter error, the file name is a valid existing file name and is a graphic. If I wait two minutes and then complete the list it works, in other words the file that seemed to cause an error, and all the subsequent files in the list, go through OK. :confused: :confused:
TheComputerMan wrote:
sourceBitmap = New Bitmap(Image.FromFile(ImagePathName))
that is bad code! Image has a Dispose() method, so you should call it; by having two operations in one line, you don't have the first image reference and can't call Dispose. Split it into two statements, and dispose. BTW: If the copy is there to avoid file lock problems, the cheaper solution is to use Image.FromStream, like so (using C# syntax):
stream=File.OpenRead(ImagePathName));
Bitmap bm=Image.FromStream(stream);
stream.Dispose();:)
Luc Pattyn
Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.
Local announcement (Antwerp region): Lange Wapper? Neen!
-
No problems on the surface, but maybe a memory leak internally... Graphics calls tend to have issues like that... Basically it wants a delegate of a particular type, in this case a
Image.GetThumbnailImageAbortdelegate. As per MSDN, you need to create a dummy function, such as:Private Function ThumbnailDummyFunc() as Boolean
End Function'And in the GetThumbnailImage call, you would pass this as the argument:
new Image.GetThumbnailImageAbort(AddressOf ThumbnailDummyFunc)(Sorry if my syntax is off... Been programming in C# all day, so it's tricky to switch mental modes)
Proud to have finally moved to the A-Ark. Which one are you in? Developer, Author (Guardians of Xen)
Hi Ian, thanks for your assistance. I resolved it!!!! :) :-D See my reply to Luc
-
TheComputerMan wrote:
sourceBitmap = New Bitmap(Image.FromFile(ImagePathName))
that is bad code! Image has a Dispose() method, so you should call it; by having two operations in one line, you don't have the first image reference and can't call Dispose. Split it into two statements, and dispose. BTW: If the copy is there to avoid file lock problems, the cheaper solution is to use Image.FromStream, like so (using C# syntax):
stream=File.OpenRead(ImagePathName));
Bitmap bm=Image.FromStream(stream);
stream.Dispose();:)
Luc Pattyn
Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.
Local announcement (Antwerp region): Lange Wapper? Neen!
Hi Luc, yes the stream thingy sorted it. I replied this to you on the other bit of the thread. Thanks for all your help.
-
Strange but true Luc, you sort of hit the nail! I did this:
sr = New StreamReader(ImagePathName) 'sourceBitmap = New Bitmap(Image.FromFile(ImagePathName)) sourceBitmap = New Bitmap(Image.FromStream(sr.BaseStream))Reading it into a stream first works - no memory errors! Thanks for you assistance.(and tutoring :) )
you're welcome. sorry, having information in two threads got me confused. It still is too expensive; I don't see why you copy the image, as
sourceBitmap = New Bitmap(Image.FromStream(sr.BaseStream))creates two identical images, the first one is the result of FromStream and is a real Image (and also a Bitmap upcast to an Image), the second the result of your Bitmap constructor is a real Bitmap. And once more, the first never gets disposed of. A simpleImage.FromStream(sr.BaseStream) As Bitmapshould do (or whatever the VB syntax is for casting from Image to Bitmap). This should halve your CPU and memory load. FYI: the person you reply to normally gets an e-mail notification; as you seem to be replying a lot to yourself, you got lots of e-mails, and I only saw your replies by watching the forum. So I suggest you make sure you reply to (the last message of) the person you want to get the reply and notification... :)Luc Pattyn
Have a look at my entry for the lean-and-mean competition; please provide comments, feedback, discussion, and don’t forget to vote for it! Thank you.
Local announcement (Antwerp region): Lange Wapper? Neen!