Preventing memory and resource leaks with GDI Objects ???
in [MFC]
|
Prev: Posting 60 messages per second to CView locks up UI (animation continues...)
Next: Turning on ClearType for TextOut() ???
From: Peter Olcott on 15 Mar 2010 11:13 Since it looks like my current code is working, (DeleteObject() consistently returns a 1 indicating successful deletion) I won't bother to explain the reasoning for my current design right now. There are a very good set of reasons that seem to not be able to be otherwise achieved. "Goran" <goran.pusic(a)gmail.com> wrote in message news:9b1a59c6-b009-47a4-b553-8bf4f986a25e(a)g10g2000yqh.googlegroups.com... On Mar 14, 6:29 pm, "Peter Olcott" <NoS...(a)OCR4Screen.com> wrote: > The problem is that the only way that I know how to > unselect > a GDI object is to select another GDI object. I can't > select > another GDI object because I have to get rid of the first > one to make room to create the second one. If there is > another way to unselect a GDI object besides selecting > another GDI object, then this would be easy. OK, then you seem to have a major case of bad understanding of the API. SelectObject returns previously selected object (of the given type). So you do: CGdiObject* pOld = pDC->SeletObject(&mySuperFont); // Draw, baby, draw... pDC->SeletObject(pOld); That's it! You don't ever, ever care about that first pOld - this baby "belongs" to a DC you are using, and is most likely one of "stock" gdi objects that are not yours to do anything with. > Ideally I only want to have a single CFont and a single > CBitmap and a single MemoryDC that I use over and over. > These must all be stored as object members, and will live > as > long as the app lives. Well, I have to ask, why all that? It doesn't seem normal. For example, if your purpose is to "draw" a bitmap for later use, then, if my memory serves me correctly, you * create a memory DC, * call CreateCompatibleBitmap, * call SelectObject for your font * draw stuff on DC * unselect the font from DC, destroy font and DC. There's your bitmap, you can keep it. If not, tell us what do you do with the three? You also seem to be misguided in wanting to reuse e.g. the font. I don't think you want it around while it's not used. If nothing else, it's a resource strain on your system. Just create it when you're drawing - what's wrong with that? Goran.
From: Joseph M. Newcomer on 15 Mar 2010 18:55 See below... On Sat, 13 Mar 2010 22:46:02 -0600, "Peter Olcott" <NoSpam(a)OCR4Screen.com> wrote: > >"Joseph M. Newcomer" <newcomer(a)flounder.com> wrote in >message news:ailop51jv036cnv0dq6po6eteuo3g7dfbv(a)4ax.com... >> Tom, >> >> you would think that in any sane system, such as one that >> uses kernel object reference >> counts [like Windows], that "deleting" a font that was >> actively selected into a DC would >> reduce the reference count to 1, so that when the font was >> deselected, the reference count >> would go to 0 and at that point the now-unused font would >> be freed and the SelectObject >> would rreturn a NULL handle. But that would probably be >> sensible, so instead, the font is >> left live and you continue execution with the impression >> that the HFONT is now referring >> to a nonexistent font, so you just ovewrite the handle and >> thus lose any access to the >> font, which continues to exist. (for HFONT, you can >> substitute HPEN, HBRUSH, HPALETTE, >> HBITMAP, etc.). >> >> Apparently, because I forgot to put the :: in front of >> ::DeleteObject, I left it ambiguous >> as to what I was talking about. CGdiObject;:DeleteObject >> produces the illusion that it >> "worked", because it allows the CFont to be reused, even >> though the actual font object in >> the kernel has not been deleted and is still occupying GDI >> kernel space. My bad. The >> truth is that ::DeleteObject will not delete (a) any GDI >> object selected into a DC (b) any >> stock object. While we don't expect stock objects to be >> deleted, the fact that objects >> can be left "dangling" always comes as a surprise, and it >> is not helped by the fact that >> CGdiObject::DeleteObject preserves the correct MFC state >> so that it *appears* that it >> worked succesfully (even though it didn't actually delete >> the object). >> joe > >So what is an example of simple clean minimal syntax for >making sure that a single set of GDI object member variables >always does get properly deleted? The best that I could >come up with is to duplicate everything such as CFont >cfont[2]; and toggle the subscript. >This seems like far too much of a kludge. **** I don't see what that can do to help. You either know what is selected, or you don't. It isn't hard to track what is selected, and you can tell immediately if you have selected it. joe **** > >> >> On Fri, 12 Mar 2010 12:25:12 -0800, "Tom Serface" >> <tom(a)camaswood.com> wrote: >> >>>Hi Peter, >>> >>>You can have different font objects if you want to have >>>multiple fonts set >>>up. You are right about not deleting it when it is >>>selected, but I don't >>>think you'd want that to happen. It typically make my >>>font objects members >>>of the class so they are always persistent while the class >>>object is... >>>Also, the assertion messages won't display for release >>>builds which is >>>convenient, but if you get them in debug you would >>>definitely want to fix >>>the problem. >>> >>>Tom >>> >>>"Peter Olcott" <NoSpam(a)OCR4Screen.com> wrote in message >>>news:hI6dnaNzGM1LiwTWnZ2dnUVZ_qSdnZ2d(a)giganews.com... >>>> >>>> "Joseph M. Newcomer" <newcomer(a)flounder.com> wrote in >>>> message >>>> news:d9jgp55513t0kb34pl3asj4qtiumk5frv0(a)4ax.com... >>>>> As I indicated in an earlier reply, you cannot call >>>>> DeleteObject on any >>>>> object which is >>>>> selected into a DC. It won't do the deletion. >>>> >>>> Apparently this is no longer true under Windows 7. I >>>> spent several hours >>>> re-arranging my code to meet your specification, and >>>> ended up having to >>>> change it back. When I tested it against your prior >>>> post: >>>> >>>> If you have an object like CFont, >>>> you cannot create more than one >>>> font in it unles you call DeleteObject >>>> explivitly, or you will get an assertion failure. >>>> >>>> and got an assertion failure when I removed the >>>> DeleteObject() for a CFont >>>> object that was currently selected into a MemoryDC. >>>> This apparently shows >>>> that DeleteObject() must work (at least for CFont) even >>>> if the CFont is >>>> currently selected into a MemoryDC. >>>> >>>> I had great difficulty trying to unselect the CFont and >>>> CBitmap objects. >>>> (see my prior post) The only way that I knew to unselect >>>> a GDI object was >>>> to select another GDI object. Since I had no other >>>> object, I tried to >>>> select an empty one. This did not work. >>>> >>>>> It is you responsibility to make sure the >>>>> objec tis not seelected. I suggest SaveDC/RestoreDC to >>>>> make sure any >>>>> object selected are >>>>> released. >>>>> joe >>>> >>>> >>>> >>>>> >>>>> >>>>> On Wed, 10 Mar 2010 17:49:56 -0600, "Peter Olcott" >>>>> <NoSpam(a)OCR4Screen.com> wrote: >>>>> >>>>>> >>>>>>"Tom Serface" <tom(a)camaswood.com> wrote in message >>>>>>news:OwfKaEKwKHA.812(a)TK2MSFTNGP06.phx.gbl... >>>>>>> Yeah, that's it... Lots of time people forget to >>>>>>> call >>>>>>> that. I turn on the GDI Objects column in Task >>>>>>> Manager so >>>>>>> I can watch what my program is doing. >>>>>> >>>>>>Can I execute CBitmap::DeleteObject() while the CBitmap >>>>>>is >>>>>>currently selected into a DC? >>>>>>(This is just before I create another CBitmap to be >>>>>>selected >>>>>>into the same DC). >>>>>> >>>>>>> >>>>>>> Tom >>>>>>> >>>>>>> "Peter Olcott" <NoSpam(a)OCR4Screen.com> wrote in >>>>>>> message >>>>>>> news:H8mdnSY2XczicgrWnZ2dnUVZ_hWdnZ2d(a)giganews.com... >>>>>>>> >>>>>>>> "Peter Olcott" <NoSpam(a)OCR4Screen.com> wrote in >>>>>>>> message >>>>>>>> news:WeWdnco19NaPcwrWnZ2dnUVZ_g2dnZ2d(a)giganews.com... >>>>>>>>> >>>>>>>>> "Tom Serface" <tom(a)camaswood.com> wrote in message >>>>>>>>> news:%23B3xq%23HwKHA.732(a)TK2MSFTNGP06.phx.gbl... >>>>>>>>>> You will have to manage the resources somehow and >>>>>>>>>> someplace, but I don't think it matters how or >>>>>>>>>> where. >>>>>>>>>> I think you get like 5000 GDI resources before >>>>>>>>>> they run >>>>>>>>>> out and I think that number is shared among >>>>>>>>>> programs. >>>>>>>>>> Still converting them is no worse that destroying >>>>>>>>>> and >>>>>>>>>> recreating them, but you'll have to destroy them >>>>>>>>>> ultimately at some point, or, if the number is not >>>>>>>>>> to >>>>>>>>>> arduous you could let them get recovered when the >>>>>>>>>> program exits (but I'm not a big fan of that >>>>>>>>>> behavior). >>>>>>>>>> >>>>>>>>>> Tom >>>>>>>>> >>>>>>>>> The main thing that I want to do is to manually >>>>>>>>> free any >>>>>>>>> memory or resources that the CBitmap object is >>>>>>>>> using. I >>>>>>>>> don't know what resources it is using, and I don't >>>>>>>>> know >>>>>>>>> how to free them. >>>>>>>>> >>>>>>>> >>>>>>>> I think that I got it: Simply call the inherited >>>>>>>> member >>>>>>>> DeleteObject(). >>>>>>>> >>>>>>>>>> >>>>>>>>>> "Peter Olcott" <NoSpam(a)OCR4Screen.com> wrote in >>>>>>>>>> message >>>>>>>>>> news:5oWdnfO3M6HKQArWnZ2dnUVZ_jCdnZ2d(a)giganews.com... >>>>>>>>>>> I am have built a general purpose bitmap/image >>>>>>>>>>> handling class and want to add TextOut() >>>>>>>>>>> capability to >>>>>>>>>>> this class. To do this I must convert some of my >>>>>>>>>>> GDI >>>>>>>>>>> local function variables into GDI object member >>>>>>>>>>> variables. This means that I must be able to >>>>>>>>>>> re-use >>>>>>>>>>> several GDI objects, instead of constructing them >>>>>>>>>>> and >>>>>>>>>>> destroying them after a single use. >>>>>>>>>>> >>>>>>>>>>> What issues are raised with resource and memory >>>>>>>>>>> leaks >>>>>>>>>>> by using CDC and CBitmap objects? How are these >>>>>>>>>>> issues mitigated? >>>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>> >>>>> Joseph M. Newcomer [MVP] >>>>> email: newcomer(a)flounder.com >>>>> Web: http://www.flounder.com >>>>> MVP Tips: http://www.flounder.com/mvp_tips.htm >>>> >>>> >> Joseph M. Newcomer [MVP] >> email: newcomer(a)flounder.com >> Web: http://www.flounder.com >> MVP Tips: http://www.flounder.com/mvp_tips.htm > Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Joseph M. Newcomer on 15 Mar 2010 18:58 What part of "use RestoreDC()" did you miss? If you simply RestoreDC() when you are done with the font, you will revert to whatever the default font was. Not a big deal. The other common practice is to note that if you do SelectObject() of a font, the result is a CFont * that had been selected and you simply re-selecte this. This technique generalizes to all 30 or so parameters of an HDC (changing a parameter returns you the prvious setting) but this means you have to keep variables around to know what value to re-selected, and I find RestoreDC() to be a simpler apprroach. joe On Sun, 14 Mar 2010 12:29:29 -0500, "Peter Olcott" <NoSpam(a)OCR4Screen.com> wrote: >The problem is that the only way that I know how to unselect >a GDI object is to select another GDI object. I can't select >another GDI object because I have to get rid of the first >one to make room to create the second one. If there is >another way to unselect a GDI object besides selecting >another GDI object, then this would be easy. > >Ideally I only want to have a single CFont and a single >CBitmap and a single MemoryDC that I use over and over. >These must all be stored as object members, and will live as >long as the app lives. > >"Goran" <goran.pusic(a)gmail.com> wrote in message >news:06b6b099-4b40-4809-868b-982febcb6066(a)t23g2000yqt.googlegroups.com... >On Mar 14, 5:46 am, "Peter Olcott" <NoS...(a)OCR4Screen.com> >wrote: >> So what is an example of simple clean minimal syntax for >> making sure that a single set of GDI object member >> variables >> always does get properly deleted? The best that I could >> come up with is to duplicate everything such as CFont >> cfont[2]; and toggle the subscript. >> This seems like far too much of a kludge. > >(I'll presume that you want to use multiple fonts to draw on >some DC). >If so, this works: > >0. create any GDI objects (e.g. fonts) >1. (optional) create your DC ( or receive it in OnPaint >:-) ) >2. select any font into DC, draw, un-select it >Rinse, repeat 2 (you can also "stack" what you do in 2 if >you wish so) >3. (optional) destroy DC (don't if it's not yours) >4. let all go out of scope (IOW, let your enclosing class be >destroyed). > >Goran. > Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Joseph M. Newcomer on 15 Mar 2010 19:02 It is very, very simple: you, and you alone, are responsible for knowing what objects are selected into a DC. If an object is selected into a DC, you must deselect it. You should not use the DC as a "repository" of any object; the simplest approach is to start with a DC which has nothing active, select into it what you need to do drawing, do the drawing, deselect all the objects (RestoreDC()), and repeat as required. It is considered poor practicce to leave an object selected into a DC across, say, an message pump invovation, because temporary objects exhibit odd behavior under such conditions. This is just good programming practice. joe On Sun, 14 Mar 2010 12:22:49 -0500, "Peter Olcott" <NoSpam(a)OCR4Screen.com> wrote: >The point is that the function listed below can be called >repeatedly, and it does not work if the > this->font.DeleteObject(); >is removed which as far as I can tell must mean that a GDI >object is definitely being deleted while selected in a >device context. Proof that a GDI object is being deleted >while selected into a device context contradicts Joe's >statement that it can't be done. > >What I suspect is there is something that I am missing here, >and Joe is not wrong, yet the explicit contradiction still >remains unresolved. > >"Goran" <goran.pusic(a)gmail.com> wrote in message >news:bc8e5ce9-773d-42df-b509-097b70e021d1(a)33g2000yqj.googlegroups.com... >On Mar 12, 4:07 pm, "Peter Olcott" <NoS...(a)OCR4Screen.com> >wrote: >> I will study your concrete example an attempt to implement >> it. My great difficulty is with the tedious little >> syntactical details, thus Joe's abstract examples don't >> work >> for me. I can't simply let my GDI object get destroyed >> because the objects must retain their state in case >> subsequent operations must be performed on them. > >I see in your code below that you use a memory DC. Without >having any >further info, only thing I have to say is that normally a >memory DC is >used to draw something, then e.g. stored in a bitmap or >BitBlt-ed on >screen. From that standpoint, normal drawing we all know is >to select >your GDI objects into the DC, call drawing code, then >un-select them. >If that is what you are doing, then you have no correlation >between >your GDI objects (e.g. bitmaps, pens, fonts, brushes) except >that they >have to outlive time when they are selected into the DC for >drawing. >So I honestly do not see your difficulty. Just make sure >that, during >the drawing code, you do stuff by the book (hence e.g. my >CTempGdiObjectSelection), and as for lifetime of your GDI >object, >well, just handle their lifetime (hence my shared_ptr-based >map >there). > >> I open a bitmap and it may be written to, or saved or >> another bitmap may be opened. If another bitmap is opened, >> I >> must create another CBitmap object of this new size. >> Currently I only have the single CBitmap object as an >> object >> member variable. > >So make that two of them. Note also that there's nothing >wrong in >creating whatever GDI object you might need on the heap. > >> I am guessing that I could implement your design by making >> pairs of GDI object member and toggling between them upon >> every re-use. For example CBitmap cbitmap[2]; and then >> toggle its subscript between one and zero. > >Here, either I am not very smart, either there's a lot of >things you >have in your head that weren't put into writing here, >because I don't >understand what you are talking about :-). > >> I looked at your design again and there were too many >> things >> that I did not understand. > >I'd say, points to take from it: > >1. use something like CTempGdiObjectSelection to ensure >correct GDI >object selection idiom, which is "select it/draw/un-select >it". > >2. use some C++ object lifetime handling technique to handle >your GDI >object (font, bitmap, pen...) lifetime. Hence I proposed >shared_ptr - >it gets you far. If you indeed use a std::map, attention to >map::operator[], that might not do what you think (hence my >insistence >on that "const"). > >3. divorce lifetime of a DC and GDI objects used to draw on >it (you >seem to be having trouble with this). In other words, when >you select >an object into a DC, do not try to destroy it. But I don't >understand >why you even started down that route. > >> Here is my currently working code. It does not work if I >> remove the: >> this->cfont.DeleteObject(); >> It does work with repeated invocations, thus must be >> deleting the selected CFont object. >> >> // Displays a Font Selection DialogBox with >> // default "Times New Roman", Bold, Italic, 8 Point >> // >> inline void ScreenCaptureType::SelectFont() { >> this->cfont.DeleteObject(); >> >> int PointSize = 8; >> int FontHeight = -MulDiv(PointSize, >> GetDeviceCaps(MemoryDC.m_hDC, LOGPIXELSY), 72); >> LOGFONT LogFont = {FontHeight, 0, 0, 0, FW_BOLD, 1, 0, 0, >> ANSI_CHARSET, OUT_DEFAULT_PRECIS, >> CLIP_DEFAULT_PRECIS, DEFAULT_QUALITY, DEFAULT_PITCH | >> FF_DONTCARE, L"Times New Roman"}; >> CFontDialog dlg(&LogFont); >> if (dlg.DoModal() == IDOK) >> cfont.CreateFontIndirect(&LogFont); >> this->MemoryDC.SelectObject(&cfont); >> >> } > >Without knowing anything about the rest of your code, the >purpose of >this function is to select some font into a memory DC >(providing that >MemoryDC indeed is a CMemoryDC). Normally, one selects a >font into a >DC to do some drawing, so I personally find it hugely >surprising that >a modal dialog is displayed just before selecting it into a >DC. I see >that working well in some bigger design only if said font is >the only >font that will be used while drawing. But, as I said before, >I don't >actually understand your difficulty at all, so I might be >completely >off the mark. > >By the way, please, PLEASE do something about that >CreateFontIndirect >call. It stands out like a sore thumb. It's a friggin >resource >allocation and that can fail (if so, it returns FALSE). >Imagine that >this indeed happens. Would you want to do >MemoryDC.SelectObject(&cfont) then? > >Goran. > Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Peter Olcott on 16 Mar 2010 08:54
"Joseph M. Newcomer" <newcomer(a)flounder.com> wrote in message news:hoetp55bqv7f9lm31e1rr5dgv41tk0rjde(a)4ax.com... > What part of "use RestoreDC()" did you miss? If you > simply RestoreDC() when you are done > with the font, you will revert to whatever the default > font was. Not a big deal. The > other common practice is to note that if you do > SelectObject() of a font, the result is a > CFont * that had been selected and you simply re-selecte > this. This technique generalizes > to all 30 or so parameters of an HDC (changing a parameter > returns you the prvious > setting) but this means you have to keep variables around > to know what value to > re-selected, and I find RestoreDC() to be a simpler > apprroach. > joe Its not just Font its also CBitmap, and there is no default bitmap to restore. It looks like all of this may be moot because DeleteObject() is confirming that the object is being deleted while it is selected by its return value of 1, so my code is already good the way that it is. I only tested this with VS 2008, but, it works on Windows 7 and XP. > > On Sun, 14 Mar 2010 12:29:29 -0500, "Peter Olcott" > <NoSpam(a)OCR4Screen.com> wrote: > >>The problem is that the only way that I know how to >>unselect >>a GDI object is to select another GDI object. I can't >>select >>another GDI object because I have to get rid of the first >>one to make room to create the second one. If there is >>another way to unselect a GDI object besides selecting >>another GDI object, then this would be easy. >> >>Ideally I only want to have a single CFont and a single >>CBitmap and a single MemoryDC that I use over and over. >>These must all be stored as object members, and will live >>as >>long as the app lives. >> >>"Goran" <goran.pusic(a)gmail.com> wrote in message >>news:06b6b099-4b40-4809-868b-982febcb6066(a)t23g2000yqt.googlegroups.com... >>On Mar 14, 5:46 am, "Peter Olcott" <NoS...(a)OCR4Screen.com> >>wrote: >>> So what is an example of simple clean minimal syntax for >>> making sure that a single set of GDI object member >>> variables >>> always does get properly deleted? The best that I could >>> come up with is to duplicate everything such as CFont >>> cfont[2]; and toggle the subscript. >>> This seems like far too much of a kludge. >> >>(I'll presume that you want to use multiple fonts to draw >>on >>some DC). >>If so, this works: >> >>0. create any GDI objects (e.g. fonts) >>1. (optional) create your DC ( or receive it in OnPaint >>:-) ) >>2. select any font into DC, draw, un-select it >>Rinse, repeat 2 (you can also "stack" what you do in 2 if >>you wish so) >>3. (optional) destroy DC (don't if it's not yours) >>4. let all go out of scope (IOW, let your enclosing class >>be >>destroyed). >> >>Goran. >> > Joseph M. Newcomer [MVP] > email: newcomer(a)flounder.com > Web: http://www.flounder.com > MVP Tips: http://www.flounder.com/mvp_tips.htm |