copy file in a thread

I am trying to write to copy a file by invoking a separate thread. Here is my form code:

unit frmFileCopy;

interface

uses
  Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
  Dialogs, ComCtrls, StdCtrls;

type
  TForm2 = class(TForm)
    Button3: TButton;
    procedure Button3Click(Sender: TObject);
    procedure FormCreate(Sender: TObject);
    procedure FormCloseQuery(Sender: TObject; var CanClose: Boolean);
  private
    ThreadNumberCounter : integer;
    procedure HandleTerminate (Sender: Tobject);

  end;

var
  Form2: TForm2;

implementation

uses
  fileThread;

{$R *.dfm}

{ TForm2 }
const
  sourcePath = 'source\'; //'
  destPath =  'dest\'; //'
  fileSource = 'bigFile.zip';
  fileDest = 'Copy_bigFile.zip';

procedure TForm2.FormCloseQuery(Sender: TObject; var CanClose: Boolean);
begin
  CanClose := true;
  if ThreadNumberCounter >0 then
  begin
    if MessageDlg('The file is being copied. Do you want to quit?', mtWarning, 
                  [mbYes, mbNo],0) = mrNo then
      CanClose := false;
  end;
end;

procedure TForm2.FormCreate(Sender: TObject);
begin
  ThreadNumberCounter := 0;
end;

procedure TForm2.Button3Click(Sender: TObject);
var
  sourceF, destF : string;
  copyFileThread : TCopyThread;
begin
  sourceF := ExtractFilePath(ParamStr(0))  + sourcePath + fileSource;
  destF := ExtractFilePath(ParamStr(0))  + sourcePath + fileDest;

  copyFileThread := TCopyThread.create(sourceF,destF);
  copyFileThread.FreeOnTerminate := True;
  try
    Inc(ThreadNumberCounter);
    copyFileThread.Execute;
    copyFileThread.OnTerminate := HandleTerminate;
    copyFileThread.Resume;
  except
    on Exception do
    begin
      copyFileThread.Free;
      ShowMessage('Error in thread');
    end;
  end;
end;

procedure TForm2.HandleTerminate(Sender: Tobject);
begin
  Dec(ThreadNumberCounter);
end;

Here is my class:

unit fileThread;

interface

uses
  Classes, SysUtils;

type
  TCopyThread = class(TThread)
  private
    FIn, FOut : string;
    procedure copyfile;
  public
    procedure Execute ; override;
    constructor create (const source, dest : string);
  end;

implementation

{ TCopyThread }

procedure TCopyThread.copyfile;
var
  streamSource, streamDest : TFileStream;
  bIn, bOut : byte;
begin
  streamSource := TFileStream.Create(FIn, fmOpenRead);
  try
    streamDest := TFileStream.Create(FOut,fmCreate);
    try
      streamDest.CopyFrom(streamSource,streamSource.Size);
      streamSource.Position := 0;
      streamDest.Position := 0;
      {check file consinstency}
      while not (streamSource.Position = streamDest.Size) do
      begin
        streamSource.Read(bIn, 1);
        streamDest.Read(bOut, 1);
        if bIn <> bOut then
          raise Exception.Create('files are different at position' +
                                 IntToStr(streamSource.Position));
      end;      
    finally
      streamDest.Free;
    end;
  finally
    streamSource.Free;
  end;
end;

constructor TCopyThread.create(const source, dest: string);
begin
  FIn := source;
  FOut := dest;
end;

procedure TCopyThread.Execute;
begin
  copyfile;
  inherited;
end;

end.

When I run the application, I received a following error:

Project prjFileCopyThread raised exception class EThread with message: 'Cannot call Start on a running or suspended thread'.

I do not have experience with threads. I use Martin Harvey's tutorial as a guide, but any advice how to improve it make safe thread would be appreciated.

---

Based on the answers, I've changed my code. This time it worked. I would appreciate if you can review it again and tell what should be improved.

procedure TForm2.Button3Click(Sender: TObject);
var
  sourceF, destF : string;
  copyFileThread : TCopyThread;
begin
  sourceF := ExtractFilePath(ParamStr(0))  + sourcePath + fileSource;
  destF := ExtractFilePath(ParamStr(0))  + destPath + fileDest;

  copyFileThread := TCopyThread.create;

  try
    copyFileThread.InFile := sourceF;
    copyFileThread.OutFile := destF;

  except
    on Exception do
    begin
   开发者_开发技巧   copyFileThread.Free;
      ShowMessage('Error in thread');
    end;
  end;

Here is my class:

type
  TCopyThread = class(TThread)
  private
    FIn, FOut : string;
    procedure setFin (const AIN : string);
    procedure setFOut (const AOut : string);
    procedure FCopyFile;
  protected
    procedure Execute ; override;
  public
    constructor Create;
    property InFile : string write setFin;
    property OutFile : string write setFOut;
  end;

implementation

{ TCopyThread }

procedure TCopyThread.FCopyfile;
var
  streamSource, streamDest : TFileStream;
  bIn, bOut : byte;
begin
  {removed the code to make it shorter}
end;

procedure TCopyThread.setFin(const AIN: string);
begin
  FIn := AIN;
end;

procedure TCopyThread.setFOut(const AOut: string);
begin
  FOut := AOut;
end;

constructor TCopyThread.create;
begin
  FreeOnTerminate := True;
  inherited Create(FALSE);
end;

procedure TCopyThread.Execute;
begin
  FCopyfile;
end;

end.

---

You have a few problems:

1. You don't call inherited Create. In this case, since you want to do things first and start it yourself, you should use

   inherited Create(True); // Creates new thread suspended.

2. You should never call Execute yourself. It's called automatically if you create non-suspended, or if you call Resume.

3. There is no inherited Execute, but you call it anyway.

BTW, you could also use the built-in Windows Shell function SHFileOperation to do the copy. It will work in the background, handles multiple files and wildcards, and can automatically display progress to the user. You can probably find an example of using it in Delphi here on SO; here is a link for using it to recursively delete files, for example.

A good search here on SO is (without the quotes) shfileoperation [delphi]

---

Just for comparison - that's how you'd do it with OmniThreadLibrary.

uses
  OtlCommon, OtlTask, OtlTaskControl;

type
  TForm3 = class(TForm)
    ...
    FCopyTask: IOmniTaskControl;
  end;

procedure BackgroundCopy(const task: IOmniTask);
begin
  CopyFile(PChar(string(task.ParamByName['Source'])), PChar(string(task.ParamByName['Dest'])), true);
  //Exceptions in CopyFile will be mapped into task's exit status
end;

procedure TForm3.BackgroundCopyComplete(const task: IOmniTaskControl);
begin
  if task.ExitCode = EXIT_EXCEPTION then
    ShowMessage('Exception in copy task: ' + task.ExitMessage);
  FCopyTask := nil;
end; 

procedure TForm3.Button3Click(Sender: TObject);
begin
  FCopyTask := CreateOmniTask(BackgroundCopy)
    .SetParameter('Source', ExtractFilePath(ParamStr(0))  + sourcePath + fileSource)
    .SetParameter('Dest', ExtractFilePath(ParamStr(0))  + destPath + fileDest)
    .SilentExceptions
    .OnTerminate(BackgroundCopyComplete)
    .Run;
end;

procedure TForm3.FormCloseQuery(Sender: TObject; var CanClose: Boolean);
begin
  CanClose := true;
  if assigned(FCopyTask) then
  begin
    if MessageDlg('The file is being copied. Do you want to quit?', mtWarning, 
                  [mbYes, mbNo],0) = mrNo then
      CanClose := false
    else
      FCopyTask.Terminate;    
  end;
end;

---

Your edited code still has at least two big problems:

• You have a parameterless constructor, then set the source and destination file names by means of thread class properties. All you have been told about creating suspended threads not being necessary holds true only if you do all setup in the thread constructor - after this has finished thread execution will begin, and access to thread properties need to be synchronized. You should (as indeed your first version of the code did) give both names as parameters to the thread. It's even worse: the only safe way to use a thread with the FreeOnTerminate property set is to not access any property once the constructor has finished, because the thread may have destroyed itself already, or could do while the property is accessed.

• In case of an exception you free the thread object, even though you have set its FreeOnTerminate property. This will probably result in a double free exception from the memory manager.

I do also wonder how you want to know when the copying of the file is finished - if there is no exception the button click handler will exit with the thread still running in the background. There is also no means of cancelling the running thread. This will cause your application to exit only when the thread has finished.

All in all you would be better off to use one of the Windows file copying routines with cancel and progress callbacks, as Ken pointed out in his answer.

If you do this only to experiment with threads - don't use file operations for your tests, they are a bad match for several reasons, not only because there are better ways to do the same in the main thread, but also because I/O bandwidth will be used best if no concurrent operations are attempted (that means: don't try to copy several files in parallel by creating several of your threads).

---

The Execute method of a thread is normally not explicitly called by client code. In other words: delete CopyFileThread.Execute in unit frmFileCopy. The thread is started when the Resume method is invoked.

Also in unit fileThread in the constructor of TCopyThread inherited Create(True) should be called as first to create a thread in suspended state.

---

You execute the thread and then trying to Resume it while it is running.

copyFileThread.Execute;
copyFileThread.OnTerminate := HandleTerminate;
copyFileThread.Resume;