Wątki i rezerwacja np ProgressBara

Chciałbym się zapytać czy poniższy kod który ma służyć rezerwacji progressbara jest poprawny. Czyli. Kilka wątków używa tego samego progressbara ale tylko jeden może go używać w tym samym czasie.

var
  Gauge_Busy : Boolean = False;
  Gauge_SetReservationOn: Boolean = False;
  rGauge : TrGauge;

{ Zwalnia dostęp do progress bara }
procedure TrGauge.EndReservation;
begin
  Gauge_Busy := False;
end;


{ Rezerwuje dostęp do gauge, jesli true to kolejne wywołanie zwróci false }
function TrGauge.Reservation: boolean;
begin
  Result := False;
  if Gauge_SetReservationOn then
    Exit;
  Gauge_SetReservationOn := True;
  if not Gauge_Busy then
  begin
    Gauge_Busy := True;
    Result := True;
  end;
  Gauge_SetReservationOn := False;
end;

A tu jak watki uzyskują dostęp do progress bara lub gauge;

procedure TTHread.Execute;
begin
  while not Terminated do
  begin
    WaitForSingleObject(hEvent, INFINITE);
    if Terminated then
      Exit;
    { Zarezerwój gauge }
    while not rGauge.Reservation do
    begin
      sleep(100); // Czekaj tak długo aż progressbar sie zwolni
      if Terminated then
        Exit;
    end;
    
    { I teoretycznie od tego momentu mozemy używać progressbara majac pewność że nikt inny nam tego nie zajmie bo  
      rGauge.Reservation zawsze  zwróci False; }


   rGauge.EndReservation; //Zwolnij progressbar;
  end;
end;  
 

I teraz pytanie czy ten sposób jest bezpieczny i prawidłowy. Pytam się gdyż natrafiłem w swojej aplikacji na błędy i czasami mogę być spowodowane tym że 2 wątki chcę w tym samym czasie korzystać z progresbara.

A czytałeś cokolwiek na temat synchronizacji wątków?

Dokładnie tak jak pisze @kAzek - dostęp do paska postępu powinien być synchronizowany, żeby wykluczyć kolizje; Twój kod widać jest jakąś ręczną implementacją, ale to tak nie działa;

Przeczytaj ten artykuł: Rozdzial 8. Aplikacje wielowątkowe i punkt o synchronizacji.

Dziękuje za wskazówkę. Ale jeśli macie na myśli synchronizacje samego progress bara to tak wykonuje. NP:

//W wątku

var
 Liczba : integer;

procedure TTHread.Execute;
begin
  while not Terminated do
  begin
    WaitForSingleObject(hEvent, INFINITE);
    if Terminated then
      Exit;
    { Zarezerwój gauge }
    while not rGauge.Reservation do
    begin
      sleep(100); // Czekaj tak długo aż progressbar sie zwolni
      if Terminated then
        Exit;
    end;
 
    { I teoretycznie od tego momentu mozemy używać progressbara majac pewność że nikt inny nam tego nie zajmie bo  

      rGauge.Reservation zawsze  zwróci False; }

   liczba := 100;
   Synchronize(Progress);

 
 
   rGauge.EndReservation; //Zwolnij progressbar;
  end;
end;  

procedure TTHread.Progress;
begin
  form1.gauge.progress := liczba;
end;

Z drugiej zaś strony może wam chodzi o synchronizacje procedury którą wymyśliłem czyli

Synchronize(Reservation);

Jedyne co trzeba synchronizować to metodę, która ma zmieniać/odczytywać właściwości; Więc Twoje rezerwacje są zbędne - załatwia to metoda Synchronize i dba o to, by kolizje nie wystąpiły.

No nie do końca. Rezerwacje są mi potrzebne bo co z tego ze synchronize załatwia aby dane się nie kolidował. Jak progres bar wizualnie będzie wyświetlał bzdury. tzn jeden wątek liczy od od 100 do 0 i wyświetla to w progress barze a drugi od 0 do 100. I oba działają jednocześnie to progress bar Value będzie przyjmowało mniej więcej takie wartości:

0,100,1,99,2,98 itp.

A tak mogę zarezerwować progress dla danego wątku a gdy wątek będzie kończył działanie i przechodził w stan "uśpienia" zwolni rezerwacji co spowoduje że inny watek może już użyć progress bara.

No ale chodzi o to że 1 wątek się wykonuje używając ProgressBara kiedy skończy to uruchamia się 2 wątek i używa tego ProgressBara?

Tak prawie. Bo może się zdarzyć że oba wątki uruchomią się w tym samym czasie i teraz muszę mieć jakiś bezpieczny sposób aby jeden wątek używał progress a drugi czekał az ten pierwszy zwolni progress bara i dopiero wtedy go zaczął używać.

Nie wiem czy o to chodzi ale zobacz przykładowy kod:

type
  TUpdateProgres = procedure(Sender: TObject; Pos: Integer) of object;

  TMyThread1 = class(TThread)
  private
    pos: Integer;
    hSem: THandle;
    fProgressProc: TUpdateProgres;
    procedure DoUpdateProgres;
  protected
    procedure Execute; override;
  public
    constructor Create(AProgressProc: TUpdateProgres);
  end;

  TMyThread2 = class(TThread)
  private
    pos: Integer;
    hSem: THandle;
    fProgressProc: TUpdateProgres;
    procedure DoUpdateProgres;
  protected
    procedure Execute; override;
  public
    constructor Create(AProgressProc: TUpdateProgres);
  end;

  TForm1 = class(TForm)
    Button1: TButton;
    ProgressBar1: TProgressBar;
    procedure Button1Click(Sender: TObject);
  private
    { Private declarations }
    procedure UpdateProgress(Sender: TObject; Pos: Integer);
  public
    { Public declarations }
  end;

const
   SEMAPHORE_NAME = 'WAIT_FOR_PROGRESS';

var
  Form1: TForm1;

implementation

{$R *.dfm}

constructor TMyThread1.Create(AProgressProc: TUpdateProgres);
begin
  inherited Create(False);
  FreeOnTerminate:= True;
  fProgressProc:= AProgressProc;
end;

procedure TMyThread1.DoUpdateProgres;
begin
  fProgressProc(Self, pos);
end;

procedure TMyThread1.Execute;
var
  i: Integer;
begin
  hSem:= CreateSemaphore(0, 1, 1, SEMAPHORE_NAME);
  WaitForSingleObject(hSem, INFINITE);
  try
    for i:=0 to 100 do
    begin
      pos:=i;
      if Assigned(fProgressProc) then
        Synchronize(DoUpdateProgres);
      Sleep(100);
      end;
  finally
    ReleaseSemaphore(hSem, 1, nil);
    CloseHandle(hSem);
  end;
end;

constructor TMyThread2.Create(AProgressProc: TUpdateProgres);
begin
  inherited Create(False);
  FreeOnTerminate:= True;
  fProgressProc:= AProgressProc;
end;

procedure TMyThread2.DoUpdateProgres;
begin
  fProgressProc(Self, pos);
end;

procedure TMyThread2.Execute;
var
  i: Integer;
begin
  hSem:= CreateSemaphore(0, 1, 1, SEMAPHORE_NAME);
  WaitForSingleObject(hSem, INFINITE);
  try
    for i:=100 downto 0 do
    begin
      pos:=i;
      if Assigned(fProgressProc) then
        Synchronize(DoUpdateProgres);
      Sleep(100);
      end;
  finally
    ReleaseSemaphore(hSem, 1, nil);
    CloseHandle(hSem);
  end;
end;

procedure TForm1.UpdateProgress(Sender: TObject; Pos: Integer);
begin
  ProgressBar1.Position:= Pos;
end;

procedure TForm1.Button1Click(Sender: TObject);
var
  MyThread1: TMyThread1;
  MyThread2: TMyThread2;
begin
  MyThread1:= TMyThread1.Create(UpdateProgress);
  MyThread2:= TMyThread2.Create(UpdateProgress);
end;

**kAzek ** z tym CreateSemaphore to był świetny pomysł. Dokładnie o coś takiego mi chodziło