Watch, Follow, &
Connect with Us
Public Report
Report From: Delphi-BCB/VCL/Graphics    [ Add a report in this area ]  
Report #:  55871   Status: Closed
TJPEGImage.Draw() is not thread safe
Project:  Delphi Build #:  XE, ... XE5
Version:    19.0 Submitted By:   Jan Derk
Report Type:  Crash / Data loss / Total failure Date Reported:  12/12/2007 1:55:58 AM
Severity:    Serious / Highly visible problem Last Updated: 4/15/2014 6:50:36 PM
Platform:    All versions Internal Tracking #:   8762
Resolution: Fixed (Resolution Comments) Resolved in Build: : XE6
Duplicate of:  None
Voting and Rating
Overall Rating: (5 Total Ratings)
5.00 out of 5
Total Votes: 97
Description
When using TJPEGImage.Draw in a thread the DC of its Bitmap.canvas is sometimes cleared due to the GDI Object Caching mechanism in graphics.pas.

This is the current code:

procedure TJPEGImage.Draw(ACanvas: TCanvas; const Rect: TRect);
begin
  ACanvas.StretchDraw(Rect, Bitmap);
end;

The solution is to add a lock on bitmap.canvas before calling stretchdraw. This is what it should be:

procedure TJPEGImage.Draw(ACanvas: TCanvas; const Rect: TRect);
begin
  Bitmap.Canvas.Lock; // New
  try
    ACanvas.StretchDraw(Rect, Bitmap);
  finally
    Bitmap.Canvas.Unlock;
  end;
end;

Note that the lock is put on the source canvas that belongs to TJPEGImage. The callers is still responsible for putting a lock on the target canvas. I mention this as some newsgroup users where confused by this.

I haven't D2007 installed so hopefully somebody else can double check if the lock is still missing in the D2007 jpeg.pas.
Steps to Reproduce:
Use TJPEGImage.Draw() in a thread. Use the interface to trigger GDI caching in the main thread and wait for empty bitmaps.
Workarounds
Add a lock to the Bitmap.Canvas in TJPEGImage.Draw()

procedure TJPEGImage.Draw(ACanvas: TCanvas; const Rect: TRect);
begin
  Bitmap.Canvas.Lock; // New line
  try
    ACanvas.StretchDraw(Rect, Bitmap);
  finally
    Bitmap.Canvas.Unlock;  // New line
  end;
end;
Attachment
JPEGThreadBug.zip
Comments

Alexey Beloborodov at 11/16/2010 9:13:12 PM -
The report is open for years and has obvious way to fix. Why CG ignores it?

Tomohiro Takahashi at 11/17/2010 6:59:19 PM -
>[Steps]
> Use TJPEGImage.Draw() in a thread. Use the interface to trigger GDI caching in the main thread and wait for empty bitmaps.
Do you have any sample project to comfirm the not-thread safe issue?
If yes, could you please upload it(as a .zip) to Discussion Forum?
[Embarcadero Discussion Forums >> Attachments]
https://forums.embarcadero.com/forum.jspa?forumID=2

Alexey Beloborodov at 11/21/2010 6:22:58 AM -
Yes. See

Test project for QC 55871.
https://forums.embarcadero.com/thread.jspa?messageID=301631&tstart=0#301631

Press Run and see normal program flow.
Check Show Bug and see how program hangs up after some seconds.

Tomohiro Takahashi at 11/21/2010 5:23:57 PM -
I attached it to this report as Sysop.
Thanks.

wang xinghua at 3/14/2011 5:33:26 PM -
yes,I just meet this gdi leak.
I avoided it like this:

G:TJpegImage;
B:TBitmap;

type
  TJpegImage_Temp=class(TJpegImage);

B.Canvas.Lock;
try
  TJpegImage_Temp(G).Bitmap.Canvas.Lock;
  try
    B.Canvas.Draw(0,0,G);
  finally
    TJpegImage_Temp(G).Bitmap.Canvas.UnLock;
  end;
finally
  B.Canvas.UnLock;
end;

Carl-Henrik Nilsson at 12/16/2013 12:37:18 PM -
Still present in XE5 update2

Tomohiro Takahashi at 12/16/2013 4:48:18 PM -
Thanks for the notification. I updated the internal status of this report.

Tomohiro Takahashi at 3/25/2014 6:02:15 PM -
The Bitmap/Canvas property for TJPEGImage will be public with next relase.

Server Response from: ETNACODE01