16

I have a background thread which loads images (either from disk or a server), with the goal of eventually passing them to the main thread to draw. When this second thread is loading GIF images using the VCL's TGIFImage class, this program sometimes leaks several handles each time the following line executes in the thread:

m_poBitmap32->Assign(poGIFImage);

That is, the just-opened GIF image is being assigned to a bitmap owned by the thread. None of these are shared with any other threads, i.e. are entirely localised to the thread. It is timing-dependent, so doesn't occur every time the line is executed, but when it does occur it happens only on that line. Each leak is one DC, one palette, and one bitmap. (I use GDIView, which gives more detailed GDI information than Process Explorer.) m_poBitmap32 here is a Graphics32 TBitmap32 object, but I have reproduced this using plain VCL-only classes, i.e. using Graphics::TBitmap::Assign.

Eventually I get an EOutOfResources exception, probably indicating the desktop heap is full:

:7671b9bc KERNELBASE.RaiseException + 0x58
:40837f2f ; C:\Windows\SysWOW64\vclimg140.bpl
:40837f68 ; C:\Windows\SysWOW64\vclimg140.bpl
:4084459f ; C:\Windows\SysWOW64\vclimg140.bpl
:4084441a vclimg140.@Gifimg@TGIFFrame@Draw$qqrp16Graphics@TCanvasrx11Types@TRectoo + 0x4a
:408495e2 ; C:\Windows\SysWOW64\vclimg140.bpl
:50065465 rtl140.@Classes@TPersistent@Assign$qqrp19Classes@TPersistent + 0x9
:00401C0E TLoadingThread::Execute(this=:00A44970)

How do I solve this and safely use TGIFImage in a background thread?

And secondly, will I encounter this same problem with the PNG, JPEG or BMP classes? I haven't so far, but given it's a threading / timing issue that doesn't mean I won't if they use similar code to TGIFImage.

I am using C++ Builder 2010 (part of RAD Studio.)


More details

Some research showed I'm not the only person to encounter this. To quote from one thread,

Help (2007) says: In multi-threaded applications that use Lock to protect a canvas, all calls that use the canvas must be protected by a call to Lock. Any thread that does not lock the canvas before using it will introduce potential bugs.

[...]

But this statement is absolute false: you MUST lock the canvas in secondary thread even if other threads don't touch it. Otherwise the canvas's GDI handle can be freed in main thread as unused at any moment (asynchronously).

Another reply indicates something similar, that it may be to do with the GDI object cache in graphics.pas.

That's scary: an object created and used entirely in one thread can have some of its resources freed asynchronously in the main thread. Unfortunately, I don't know how to apply the Lock advice to TGIFImage. TGIFImage has no Canvas, although it does have a Bitmap which has a canvas. Locking that has no effect. I suspect that the problem is actually in TGIFFrame, an internal class. I also do not know if or how I should lock any TBitmap32 resources. I did try assigning a TMemoryBackend to the bitmap, which avoids using GDI, but it had no effect.

Reproduction

You can reproduce this very easily. Create a new VCL app, and make a new unit which contains a thread. In the thread's Execute method, place this code:

while (!Terminated) {
    TGraphic* poGraphic = new TGIFImage();
    TBitmap32* poBMP32 = new TBitmap32();
    __try {
        poGraphic->LoadFromFile(L"test.gif");
        poBMP32->Assign(poGraphic);
    } __finally {
        delete poBMP32;
        delete poGraphic;
    }
}

You can use Graphics::TBitmap if you don't have Graphics32 installed.

In the app's main form, add a button which creates and starts the thread. Add another button which executes similar code to the above (once only, no need to loop. Mine also stores the TBitmap32 as a member variable instead of creating it there, and invalidates so it will eventually paint it to the form.) Run the program and click the button to start the thread. You will probably see GDI objects leak already, but if not press the second button which runs the similar code once in the main thread - once is enough, it seems to trigger something - and it will leak. You will see memory usage rise, and that it leaks GDI handles at the rate of several dozen per second.

David
  • 12,749
  • 6
  • 61
  • 126
  • 4
    'That's scary: an object created and used entirely in one thread can have some of its resources freed asynchronously in the main thread' - indeed, very scary. Even if you know about this aparrent bug, what can you do? Even if there is a lock mechansims that works, the GUI thread may release something before you can lock it :(( – Martin James Apr 19 '12 at 15:40
  • Glad you agree it's scary :) I hope I can lock around the Assign call, since that's where the problem seems to lie; your comment about timing there though is a good point. I suppose it depends when and how resources are created. Right now, I have TGIFImage support removed in my app, but GIF images are useful and I want to put them back! I'm also worried the same thing might happen for PNG, JPEG and BMP images. – David Apr 19 '12 at 15:52
  • 2
    @David - Confirmed with D2007. I can't help with the bug but I replaced the stock 'GIFImg.pas' with 'GIFImage.pas' from 'jvcl\devtools\res2bmp' folder and the test app is still running (about 10 min). Unfortunately there are too much differences for a diff, but I guess you can make out since you know where to look. – Sertac Akyuz Apr 19 '12 at 16:25
  • Thanks Sertac! I don't have JVCL installed at the moment, but you've just given me a very good reason to download it :) You mention diffing - was the JVCL implementation originally based on [Anders Melander's TGIFImage](http://melander.dk/delphi/gifimage/), which is the origin of the VCL class? – David Apr 19 '12 at 16:30
  • @David - You're welcome. I thought so but now I'm not sure. gifimage.pas is here: http://jvcl.svn.sourceforge.net/viewvc/jvcl/trunk/jvcl/devtools/Res2Bmp/gifimage.pas?view=log – Sertac Akyuz Apr 19 '12 at 16:31
  • 4
    There is a (possible) problem with TBitmap when used inside a second thread. The Thread does not own them like you think. Check the `Graphics` unit, and read notes about `FreeMemoryContexts`/`BitmapCanvasList`. I had strange random exceptions with TBitmaps used in Thread not touching the main VCL Thread (Locking bitmap canvas or not)... maybe I don't fully understand how to use it. Also your TGifImage might be using another separate Thread to process frames... I'm totally lost with that personally, and gave it up. – kobik Apr 19 '12 at 16:51
  • 4
    BTW, My conclusion was also "you **MUST** lock the canvas in secondary thread even if other threads don't touch it." because the VCL main winproc will release memory DCs that are not locked, killing the bitmap DC! – kobik Apr 19 '12 at 17:07
  • 1
    Graphics.pas: '(garbage collection). Only memory DCs not locked by other threads will be freed.' GC! Invaded by Java! – Martin James Apr 19 '12 at 17:07
  • 1
    How can we safely lock the DC when the GUI thread is processing messages independently? It's a race beween the work thread trying to get the lock and the GUI thread trying to garbage-collect on every message. Talking of garbage, I suppose TThread.Synchronize() would help, but I don't want that s... in my code! – Martin James Apr 19 '12 at 17:12
  • ..actually, no it wouldn't help 'cos the lock would then be owned by the GUI thread :(( – Martin James Apr 19 '12 at 17:13
  • @MartinJames, That was exacally my problem . I *sometimes* got AVs from the MainWndProc when Graphics was calling `TryLock` winning the race even if my bitmap canvas was locked... – kobik Apr 19 '12 at 17:22
  • Override TBitmapCanvas.CreateHandle, copy the code but comment out the 'Unlock' in the finally? If the component is later signalled to the GUI thread, you would have to remember to remove the lock first. – Martin James Apr 19 '12 at 17:34
  • I believe the only possible solution is to use critical sections when you touch the bitmap canvas, to synchronize locking. – kobik Apr 19 '12 at 20:10
  • 1
    I think you need to find some clean GIF code rather than the VCL code. Graphics32 is clean so you are safe there. What about TWICImage which stands on top of Windows Imaging Components and won't suffer from such problems? – David Heffernan Apr 19 '12 at 22:16
  • @kobik - there is already a lock. I'm afraid that adding another lock on top invites deadlock. – Martin James Apr 20 '12 at 08:55
  • @SertacAkyuz, I just tried using the jvcl\devtools\res2bmp\GifImage.pas file, and unfortunately it leaked for me too. I also tried [another version of the old TGIFImage code](http://www.tolderlund.eu/delphi/), maintained by Finn Tolderlund. I think David Heffernan's suggestion of TWICImage might work, but unfortunately I need to support Windows 2000 too (WIC is XP SP3+.) I think I will stick with not supporting GIF at all for the moment. – David May 07 '12 at 12:23
  • @DavidM - Hi David. I bumped my head into this bug also. Some years have past since the problem was posted. Did you find a solution? :) – Z80 May 30 '17 at 10:11
  • 1
    @FreeAndNil At the time, no, but in 2017 I'd try using Windows Imaging Components (WIC) for GIF support and see how that works. There's support in Delphi. – David Jun 12 '17 at 16:14

1 Answers1

1

Unfortunately, the fix is very, very ugly. The basic idea is that the background thread must acquire a lock that the main thread holds when it's between messages.

The naive implementation is like this:

  1. Lock canvas mutex.
  2. Spawn background thread.
  3. Wait for message.
  4. Release canvas mutex.
  5. Process message.
  6. Lock canvas mutex.
  7. Go to step 3.

Note that this means the background thread can only access GDI objects while the main thread is busy, not while it's waiting for a message. And this means the background thread cannot own any canvasses while it does not hold the mutex. These two requirements tend to be too painful. So you may need to refine the algorithm.

One refinement is to have the background thread send the main thread a message when it needs to use a canvas. This will cause the main thread to more quickly release the canvas mutex so the background thread can get it.

I think this will be enough to make you give up this idea. Instead, perhaps, read the file from the background thread but process it in the main thread.

David Schwartz
  • 166,415
  • 16
  • 184
  • 259
  • Truly an ugly fix! Thanks, though. At the moment, I've simply dropped support for GIF images - I think that's preferable compared to this. I also think this would be very hard to implement, given that the leak is caused by VCL code that would need to be modified. – David Jun 13 '12 at 11:14