QualityCentral

Public Report

Report From: Delphi-BCB/VCL/Graphics [ Add a report in this area ]

Report #: 55871

Status: Open

TJPEGImage.Draw() is not thread safe

Project: Delphi

Build #: XE

Version: 15.0

Submitted By: Jan Derk

Report Type: Basic functionality failure

Date Reported: 12/12/2007 1:55:58 AM

Severity: Infrequently encountered problem

Last Updated: 12/9/2012 5:35:58 PM

Platform: All versions

Internal Tracking #: 8762

Resolution: None (Resolution Comments)

Resolved in Build: : None

Duplicate of: None

Voting and Rating

Overall Rating: (5 Total Ratings)

5.00 out of 5

Total Votes: 77

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;

Developer Tools

Database Tools