Delphi Simple Multithreading , Create File if it is not present .

Hi all,

I am trying to create a very simple thread . I want to create a file if it does not exists . And only one thread may run at a time , even if I have 5 threads waiting .

unit FilemuxThread;

interface

uses System.Classes, System.SysUtils, Winapi.Messages, VCL.Forms, Winapi.Windows;

type
  TFilemuxThread = Class(TThread)
  Public
    constructor Create(Rack: String; Glas: String; EsBC : string; XLR_Node : string);
  protected
    tRack: string;
    tGlas: string;
    tEsBC: string;
    tXLR_Node: string;

    procedure Execute; override;
  end;

var isDone : boolean;

implementation

uses inifunctions, DateUtils, MainForm;

constructor TFilemuxThread.Create(Rack: String; Glas: String; EsBC : string; XLR_Node : string);
begin
  inherited Create(False);

  FreeOnTerminate:=True;
  tRack:=Rack;
  tGlas:=Glas;
  tEsBC:=EsBC;
  tXLR_Node:=XLR_Node;


  PostMessage( Main.Handle, TH_MESSAGE, 10, 1);

end;

procedure TFilemuxThread.Execute;
var mypath : string;
    mytimestamp : TDateTime;
    myFile : TextFile;
begin
  inherited;

  try
    while isDone = false do
      begin
        if fileexists(ExtractFilePath(Application.ExeName)+'\Temp\Line5.dat') = false then
          begin
            mypath := iniread('setup.conf','options','file');
            mytimestamp:=date+time;
            AssignFile(myFile, ExtractFilePath(Application.ExeName)+'\Temp\Line5.dat');
            ReWrite(myFile);

            WriteLn(myFile,formatdatetime('yyyymmddhhnnss',myTimeStamp),':'+tEsBC+'#'+tXLR_NODE);
            myTimeStamp:=incsecond(MyTimeStamp,1);
            WriteLn(myFile,formatdatetime('yyyymmddhhnnss',myTimeStamp),':'+tGlas+'#'+tXLR_NODE);

            CloseFile(myFile);


            PostMessage( Main.Handle, TH_MESSAGE, 10, -1);

            isDone:=true;
          end;
      end;
  except
    // we eat errors for lunch :)
  end;
end;

end.

Open in new window


What am I doing wrong here?

Thanks
Robert BecskeiAsked:
Who is Participating?
 
Robert BecskeiConnect With a Mentor Author Commented:
Meanwhile I did it like this ( it is actually kinda like you told me to )

procedure TMain.Button1Click(Sender: TObject);
var mytimestamp : TDateTime;
    myFileName : String;
    myFile : TextFile;
begin

  myFileName:=formatdatetime('yyyymmddhhnnss',date+time);

  mytimestamp:=date+time;

  AssignFile(myFile, ExtractFilePath(Application.ExeName)+'\Temp\'+myFileName+'.temp');
  ReWrite(myFile);

  WriteLn(myFile,formatdatetime('yyyymmddhhnnss',myTimeStamp),':'+iniread('setup.conf','options','BDEUser')+'#'+iniread('setup.conf','options','Node'));
  myTimeStamp:=incsecond(MyTimeStamp,1);

  WriteLn(myFile,formatdatetime('yyyymmddhhnnss',myTimeStamp),':'+iniread('setup.conf','options','EsBC')+'#'+iniread('setup.conf','options','Node'));
  myTimeStamp:=incsecond(MyTimeStamp,1);

  WriteLn(myFile,formatdatetime('yyyymmddhhnnss',myTimeStamp),':'+Edit1.Text+'#'+iniread('setup.conf','options','Node'));

  CloseFile(myFile);

  RenameFile(ExtractFilePath(Application.ExeName)+'\Temp\'+myFileName+'.temp',ExtractFilePath(Application.ExeName)+'\Temp\'+myFileName+'.dat');

end;

Open in new window


File is created with info as soon as I have the barcode ... once I have done writing to file I rename it to .dat

On the timer... which runs 1 sec interwall ... I pool the folder for .dat files like this
procedure TMain.FileProcessor;
begin
  // get .dat files from dir
  Files := TDirectory.GetFiles(ExtractFilePath(Application.ExeName)+'\Temp\','*.dat');

  if length(Files) <>  0 then
    begin
      if fileexists(iniread('setup.conf','options','file')) = false then
        begin
          CopyFileAcrossNetwork(Files[0], iniread('setup.conf','options','file')+'.tmp');
          System.SysUtils.DeleteFile(Files[0]);
          RenameFile(iniread('setup.conf','options','file')+'.tmp',iniread('setup.conf','options','file'));
        end;
    end;
end;

Open in new window


This should do just fine .

Thank you
0
 
Robert BecskeiAuthor Commented:
Seems that I solved this :

I sleep before trying to check for the file ... for 1 sec , still I am not sure if this is the right way to do it . If there is a better way . Please do tell :)

procedure TFilemuxThread.Execute;
var mypath : string;
    mytimestamp : TDateTime;
    myFile : TextFile;
begin
  inherited;

  try
    while isDone = false do
      begin
        sleep(1000);
        if fileexists(ExtractFilePath(Application.ExeName)+'\Temp\Line5.dat') = false then
          begin
            mypath := iniread('setup.conf','options','file');
            mytimestamp:=date+time;
            AssignFile(myFile, ExtractFilePath(Application.ExeName)+'\Temp\Line5.dat');
            ReWrite(myFile);

            WriteLn(myFile,formatdatetime('yyyymmddhhnnss',myTimeStamp),':'+tEsBC+'#'+tXLR_NODE);
            myTimeStamp:=incsecond(MyTimeStamp,1);
            WriteLn(myFile,formatdatetime('yyyymmddhhnnss',myTimeStamp),':'+tGlas+'#'+tXLR_NODE);

            CloseFile(myFile);

            isDone:=true;

            PostMessage( Main.Handle, TH_MESSAGE, 10, -1);

          end;
      end;
  except
    // we eat errors for lunch :)
  end;
end;

Open in new window

0
 
ste5anSenior DeveloperCommented:
Drop the entire FileExists() tests. The problem is concurrency. FileExists() does not really help. It can produce false negatives as well as false positives.

Here we just access the file and use SEH (structured error handling) aka try..except to handle OS errors.

p.s. I would also read the ini file only once. When it is necessary to pickup changes to it, I would consider using a file watcher.
0
Cloud Class® Course: C++ 11 Fundamentals

This course will introduce you to C++ 11 and teach you about syntax fundamentals.

 
Robert BecskeiAuthor Commented:
ste5an , how would that help me ?
At any given time I only have one file name which I must use : Line5.dat.

This is what happens :

1. Barcode is read
2. File is created
3. Server app processes this file
4. Repeat from 1


If the impossible happens and the File is not Processed by the Server App , but I already recievedI new Barcode, I must create a thread for this new Barcode which then waits until the first one is done . And then once it is done it creates a new Line5.dat . ( this is highly unlikely , I am just being paranoid )

This is why I used FileExists ... my first idea was to check if the First Thread is running,  but right now I lack the knowledge how to do it.

What I wanted to do first , but was not successful ...
Create a Thread ... and once the Next Thread is created it will only run if the thread before it is done .
So if I start 4 threads at once , they will be executed one-by-one ... this defeats the purpose... but in this case it makes sense.
Because this file creation must not interfere with the main process , and must be done in order .

Any tips on this ?
0
 
Geert GOracle dbaCommented:
FileExists doesn't always work, I second ste5an on that
we changed our file processor, to actually check 3 times (something with a DNS failure the first time)

besides that, you have circular unit reference
your postmessage solution is half way there
when creating the thread, provide the handle of the form receiving the message

type
  TFilemuxThread = Class(TThread)
  Public
    constructor Create(Receiver: THandle: THandle; Rack: String; Glas: String; EsBC : string; XLR_Node : string);
  protected
    fReceiver: THandle;
    tRack: string;
    tGlas: string;
    tEsBC: string;
    tXLR_Node: string;

    procedure Execute; override;
  end;

var isDone : boolean;

implementation

uses inifunctions, DateUtils;

constructor TFilemuxThread.Create(Receiver: THandle; Rack: String; Glas: String; EsBC : string; XLR_Node : string);
begin
  inherited Create(False);

  FreeOnTerminate:=True;
  tRack:=Rack;
  tGlas:=Glas;
  tEsBC:=EsBC;
  tXLR_Node:=XLR_Node;
  fReceiver := Receiver;

  PostMessage( fReceiver, TH_MESSAGE, 10, 1);

end;

Open in new window

0
 
Robert BecskeiAuthor Commented:
Thank you. I think I finally got what I wanted . I tested this one with 30 Threads , and it does em sequentially as I wished for .

On the MainForm , I have a         ActiveFileMuxThreadID : integer; , which is used to keep count of the active stuff.
Each time a Thread finished it sends a message to the main form which then increments the number .

procedure TMain.MessageReceiver(var msg: TMessage);
begin
// later we will log the errors only
  if msg.WParam = 10 then // info from FileMux Thread
    begin
      if msg.LParam = 2 then inc(Main.ActiveFileMuxThreadID);
      if msg.LParam = -1 then
        begin
          // log the error in text file and continue with next one
          inc(Main.ActiveFileMuxThreadID);

        end;

    end;
end;

Open in new window


I tested it like this
  Filemux := TFilemuxThread.Create(0,'','1');
  Filemux := TFilemuxThread.Create(1,'','2');
  Filemux := TFilemuxThread.Create(2,'','3');
  Filemux := TFilemuxThread.Create(3,'','4');
  Filemux := TFilemuxThread.Create(4,'','5');

  Filemux := TFilemuxThread.Create(5,'','6');
  Filemux := TFilemuxThread.Create(6,'','7');
  Filemux := TFilemuxThread.Create(7,'','8');
  Filemux := TFilemuxThread.Create(8,'','9');
  Filemux := TFilemuxThread.Create(9,'','10');

  Filemux := TFilemuxThread.Create(10,'','11');
  Filemux := TFilemuxThread.Create(11,'','12');
  Filemux := TFilemuxThread.Create(12,'','13');
  Filemux := TFilemuxThread.Create(13,'','14');
  Filemux := TFilemuxThread.Create(14,'','15');

  Filemux := TFilemuxThread.Create(15,'','16');
  Filemux := TFilemuxThread.Create(16,'','17');
  Filemux := TFilemuxThread.Create(17,'','18');
  Filemux := TFilemuxThread.Create(18,'','19');
  Filemux := TFilemuxThread.Create(19,'','20');

  Filemux := TFilemuxThread.Create(20,'','21');
  Filemux := TFilemuxThread.Create(21,'','22');
  Filemux := TFilemuxThread.Create(22,'','23');
  Filemux := TFilemuxThread.Create(23,'','24');
  Filemux := TFilemuxThread.Create(24,'','25');

  Filemux := TFilemuxThread.Create(25,'','26');
  Filemux := TFilemuxThread.Create(26,'','27');
  Filemux := TFilemuxThread.Create(27,'','28');
  Filemux := TFilemuxThread.Create(28,'','29');
  Filemux := TFilemuxThread.Create(29,'','30');

Open in new window


My original post failed by this ... this one works . I tested it now multiple times . I post the message to the Mainform , because I would
like to keep count of the threads as well, in case something bad happens .


The Thread unit .  There is no circular reference because the Main form is referenced under implementation . Not at the top .

unit FilemuxThread;

interface

uses System.Classes, System.SysUtils, Winapi.Messages, VCL.Forms, Winapi.Windows;

type
  TFilemuxThread = Class(TThread)
  Public
    constructor Create(ID : integer;Rack: String; Glas: String);
  protected
    tID : integer;

    tRack: string;
    tGlas: string;

    isDone : boolean;

    procedure Execute; override;
  end;

implementation

uses inifunctions, DateUtils, MainForm;

function CopyFileAcrossNetwork(source, dest : string): boolean;
begin
   Result := CopyFile(PChar(source), pchar(dest), FALSE);
end;


constructor TFilemuxThread.Create(ID : integer;Rack: String; Glas: String);
begin
  inherited Create(False);

  FreeOnTerminate:=True;

  tID:=ID;
  tRack:=Rack;
  tGlas:=Glas;

  isDone := false;

  PostMessage( Main.Handle, TH_MESSAGE, 10, 1);

end;
procedure TFilemuxThread.Execute;
var mypath : string;
    mytimestamp : TDateTime;
    myFile : TextFile;
begin
  inherited;

  try
    while (tID <> Main.ActiveFileMuxThreadID) do
      begin
        sleep(1000); // we just hang on here until it is our turn
      end;


    while isDone = false do
      begin
        if fileexists(iniread('setup.conf','options','file')) = false then
          begin
            mypath := iniread('setup.conf','options','file');
            mytimestamp:=date+time;
            AssignFile(myFile, ExtractFilePath(Application.ExeName)+'\Temp\temp.dat');
            ReWrite(myFile);

            WriteLn(myFile,formatdatetime('yyyymmddhhnnss',myTimeStamp),':'+iniread('setup.conf','options','BDEUser')+'#'+iniread('setup.conf','options','Node'));
            myTimeStamp:=incsecond(MyTimeStamp,1);

            WriteLn(myFile,formatdatetime('yyyymmddhhnnss',myTimeStamp),':'+iniread('setup.conf','options','EsBC')+'#'+iniread('setup.conf','options','Node'));
            myTimeStamp:=incsecond(MyTimeStamp,1);

            WriteLn(myFile,formatdatetime('yyyymmddhhnnss',myTimeStamp),':'+tGlas+'#'+iniread('setup.conf','options','Node'));

            CloseFile(myFile);

            CopyFileAcrossNetwork(ExtractFilePath(Application.ExeName)+'\Temp\temp.dat', iniread('setup.conf','options','file')+'.tmp');
            System.SysUtils.DeleteFile(ExtractFilePath(Application.ExeName)+'\Temp\temp.dat');
            RenameFile(iniread('setup.conf','options','file')+'.tmp',iniread('setup.conf','options','file'));

            isDone:=true;

            PostMessage( Main.Handle, TH_MESSAGE, 10, 2);

          end;
      end;
  except
    PostMessage( Main.Handle, TH_MESSAGE, 10, -1);
  end;
end;
end.

Open in new window

0
 
ste5anSenior DeveloperCommented:
There should be no reference to the main form at all. Pass the necessary values as ctor parameter. Instead of sleep I would use WaitForMultipleObjects. This also allows to externally trigger your thread loop execution.

Race condition:

 if fileexists(iniread('setup.conf','options','file')) = false then
          begin
            mypath := iniread('setup.conf','options','file');

Open in new window

The value can change between those two lines.

And using a mutex to protect your thread execution would be then most common way to ensure that only one thread is executing.
0
 
Geert GOracle dbaCommented:
besides that, you are assuming that main. is the name of the instance of TMain
it's way better to use Self.

it doesn't happen often that people create multiple instances of the main form
but other forms, yes, that can happen often

in case something bad happens ?

like a deadlock ? multiple threads waiting for the same resource ?
you can't catch or fix that with a counter

your thread unit should work without any reference to the main unit:

uses inifunctions, DateUtils, MainForm;

remove it from the implementation uses list
0
 
Geert GOracle dbaCommented:
have you tested running your app on multiple computers simultaneously ?
0
 
Robert BecskeiAuthor Commented:
Hi I Read The Famous Manual ,

Per Program there is only 1 file . So multiple apps on multiple computers no problem.
When I will have 5 programms , all five use different files .
1.dat Programm1
2.dat Programm2
3.dat Programm3
4.dat Programm4
5.dat Programm5

The main form can only be ran once, this is a Barcode Scanner system , the user cannot do anything , everything is password protected. This is nothing more then a display + logging system .

So I understand , I am not doing everything by the book ... I know , I am pretty new to threading but I will learn .

I can only say : this works, "I think" I understand why it works and how it works :) . I tried yesterday with 100 threads ... it still worked flawlessly.
This might be bad code ... but I have to begin somewhere .

But if I may ask something ( and this is not a homework stuff I am doing ) . How would you do this ? How would you make sure that only one thread is running at a time and sequentially . With a very small example . Not just code fragments , and tips .

Thank you.
0
 
ste5anSenior DeveloperCommented:
Okay, " Barcode Scanner system". What is the use-case for your threading here?

A barcode scanning system needs three threads: The main UI thread, to keep the UI responsive. Then a scanner thread to ensure that it captures all scanned codes. And a storage thread who stores all scanned data.
0
 
Robert BecskeiAuthor Commented:
Between Scans I have approx 1 min . So there is only one Main Thread ... which has the Comport component.
1. Barcode Scanned
2. Item identified  
(1 and 2 no need to be in Thread , because if they fail it means the entire production is dead anyway ... )
3. Special File Created from the Barcode for the Booking Service . ( this is the only thread , and only in case I somehow manage to overload the Booking Service ... highly unlikely ... but still I wanted to make a thread , in sequence , which then tries to slowly insert the jobs )

It seems like I am bugging out the Comport component , because if I create the threads with a button it works ...
If I create the thread with the Comport .ComPortRxChar it somehow manages to screw it up . I get one reading and after that the comport stays open .
0
 
ste5anConnect With a Mentor Senior DeveloperCommented:
aha. When using a scanner attached to a COM port, you need exactly one thread for the scanning. Cause COM ports are not a shareable resource. Also they are serial communication devices, thus one thread is enough.

The general outline is pretty simple, once you understood this:

One scanner thread reads barcodes. It keeps the COM port open to get all data send to it. Each read barcode is stored in queue. Then you can have one or multiple worker threads who query the queue. And do their work, when they get a barcode from the queue. Multiple worker threads make sense, when processing of the barcode is slower than new barcodes arrive in the queue.
0
 
Robert BecskeiAuthor Commented:
I think I am doing an overkill here ... I will do this
1. Barcode scanned
2. Barcode saved to file scanned_queue.dat ( barcode, scanntime )
3. On the main timer which runs every second ... I parse this file ...
4. while not eof file
     if fileexists on server <> 0
     then create the new file from the actual line in scanned_queue.dat
     and delete this line.
5. repeat

It is highly unlikely that this text file will every have more then 10 lines... I will try this first .
0
 
ste5anConnect With a Mentor Senior DeveloperCommented:
I would not use a single file solution. File concurrency issues are problematic.

Here the most common solution is using semaphores. Thus one file per barcode. Having two folders BarcodesScanned, BarcodesProcessed (the processed folder is a kind of archive/log).
When a barcode is scanned, a new file having the barcode in the filename is created in the scanned folder and placed in the queue object. When a barcode is processed the file is moved from scanned to processed.

Thus the semaphore files are a kind of transaction log for the scan, queue and processing instances in you application.

Advantage of this solution: you always see with a simple DIR on the folder, what barcodes are still pending to be processed.

It is highly unlikely that this text file will every have more then 10 lines... I will try this first
I would not rely on probability.
0
 
Robert BecskeiAuthor Commented:
A typical case of Noob doing a overkill for something ...
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.