4

First of all I would like to show you my code:

unit BSort;

{==============================================================================}

{$mode objfpc}{$H+}

{==============================================================================}

interface

{==============================================================================}

uses
  Classes, SysUtils;

{==============================================================================}

type
  TcompFunc = function(AValue1, AValue2 : Integer) : boolean;
  TIntegerArray = array of integer;
  PIntegerArray = ^TIntegerArray;

{==============================================================================}

procedure BubbleSort(var AMatrix : TIntegerArray; ACompFunc : TCompFunc);
function V1LargerV2(AValue1, AValue2 : Integer) : Boolean;

{==============================================================================}

implementation

{==============================================================================}

procedure Swap(var AValue1, AValue2 : Integer);
var
  Tmp : Integer;
begin
  Tmp := AValue1;
  AValue1 := AValue2;
  AValue2 := Tmp;
end;

{==============================================================================}

function V1LargerV2(AValue1, AValue2 : Integer) : Boolean;
begin
  result := AValue1 > AValue2;
end;

{------------------------------------------------------------------------------}

procedure BubbleSort(var AMatrix : TIntegerArray; ACompFunc : TCompFunc);
var
  i,j : Word;
begin
  for i := Low(AMatrix) to High(AMatrix) - 1 do
    for j := Low(AMatrix) to High(AMatrix) - 1 do
    begin
      if ACompFunc(AMatrix[j], AMatrix[j+1]) then
        Swap(AMatrix[j], AMatrix[j+1]);
    end;
end;

{==============================================================================}

end.

unit MultiThreadSort;

{==============================================================================}

{$mode objfpc}{$H+}

{==============================================================================}

interface

{==============================================================================}

uses
  Classes, SysUtils, BSort;

{==============================================================================}

type
  TSortThread = class(TThread)
      FMatrix : PIntegerArray;
    protected
      procedure Execute; override;
    public
      constructor Create(var AMatrix : TIntegerArray);
    public
      property Terminated;
  end;

{==============================================================================}

implementation

{==============================================================================}

constructor TSortThread.Create(var AMatrix : TIntegerArray);
begin
  inherited Create(False);
  FreeOnTerminate := False;
  FMatrix := @AMatrix;
end;

{------------------------------------------------------------------------------}

procedure TSortThread.Execute;
begin
  BubbleSort(FMatrix^, @V1LargerV2);
end;

{==============================================================================}

end.


program sortuj;

{==============================================================================}

{$mode objfpc}{$H+}

{==============================================================================}

uses
  {$IFDEF UNIX}{$IFDEF UseCThreads}
  cthreads,
  {$ENDIF}{$ENDIF}
  Classes, SysUtils, MultiThreadSort, BSort, Crt;

{==============================================================================}

const
  Zakres = 20;

{==============================================================================}

var
  Start  : Double;
  Stop   : Double;
  Time   : array[0..1] of Double;
  Matrix : array[0..9] of TIntegerArray;
  i,j    : Word;

{==============================================================================}

procedure Sort(var AMatrix : TIntegerArray);
var
  SortThread : array[0..1] of TSortThread;
  Matrix     : array[0..1] of TIntegerArray;
  Highest    : Integer;
  i, j, k    : Word;
begin
  // Znalezienie największej liczby w tablicy.
  Highest := Low(Integer);
  for i := Low(AMatrix) to High(AMatrix) do
    if AMatrix[i] > Highest then
      Highest := AMatrix[i];

  // Zerowanie tablic pomocniczych.
  for i := 0 to 1 do
    SetLength(Matrix[i], 0);

  // Podział tablicy do sortowania na dwie tablice:
  // - pierwsza od najniższej do połowy najwyższej liczby.
  // - druga od połowy najwyższej do najwyższej liczby.
  j := 0;
  k := 0;
  for i := Low(AMatrix) to High(AMatrix) do
    if AMatrix[i] < Highest div 2 then
    begin
      SetLength(Matrix[0], Length(Matrix[0]) + 1);
      Matrix[0,j] := AMatrix[i];
      Inc(j);
    end
    else
    begin
      SetLength(Matrix[1], Length(Matrix[1]) + 1);
      Matrix[1,k] := AMatrix[i];
      Inc(k);
    end;

  //Tworzenie i start wątków sortujacych.
  for i := 0 to 1 do
    SortThread[i] := TSortThread.Create(Matrix[i]);

  // Oczekiwanie na zakończenie watków sortujących.
  //for i := 0 to 1 do
  //  SortThread[i].WaitFor;
  //  while not SortThread[i].Terminated do
  //    sleep(2);

  Sleep(10);
  SortThread[0].WaitFor;
  Sleep(10);
  SortThread[1].WaitFor;
  Sleep(10);

  // Zwalnianie wątków sortujacych.
  for i := 0 to 1 do
    FreeAndNil(SortThread[i]);

  // Łączenie tablic pomocniczych w jedną.
  k := 0;
  for i := 0 to 1 do
    for j := Low(Matrix[i]) to High(Matrix[i]) do
    begin
      AMatrix[k] := Matrix[i,j];
      Inc(k);
    end;
end;

{==============================================================================}

begin
  Randomize;
  ClrScr;

  for i := 0 to 9 do
  begin
    SetLength(Matrix[i],Zakres);
    Write('Losowanie ', i, ' tablicy...');
    for j := 0 to Zakres - 1 do
      Matrix[i,j] := Random(100) - 50;
    Writeln('Wylosowana');
  end;

  Writeln;
  Start := TimeStampToMsecs(DateTimeToTimeStamp(Now));
  for i := 0 to 9 do
  begin
    Write('Sortowanie ', i, ' tablicy...');
    BubbleSort(Matrix[i],@V1LargerV2);
    Writeln('Posortowana');
  end;
  Stop := TimeStampToMsecs(DateTimeToTimeStamp(Now));
  Time[0] := Stop - Start;

  Writeln;
  for i := 0 to 9 do
  begin
    Write('Losowanie ',i,' tablicy...');
    for j := 0 to Zakres do
      Matrix[i,j] := Random(100) - 50;
    Writeln('Wylosowana');
  end;

  Writeln;
  Start := TimeStampToMsecs(DateTimeToTimeStamp(Now));
  for i := 0 to 9 do
  begin
    Write('Sortowanie dwuwatkowe ', i, ' tablicy...');
    Sort(Matrix[i]);
    Writeln('Posortowana');
  end;
  Stop := TimeStampToMsecs(DateTimeToTimeStamp(Now));
  Time[1] := Stop - Start;

  Writeln;
  Writeln('Sortowanie bąbelkowe : ',Time[0]);
  Writeln('Sortowanie dwuwatkowe: ',Time[1]);
  Readln;
end.

When I compile that code and run with Delphi 7 it is working fine. But when I compile it with Lazarus the last "writeln" text is doubled or tripled and program hangs. Could someone tell me why?

Delphi 7 is correct: Delphi 7

Lazarus is not correct: Lazarus

Peter O.
  • 32,158
  • 14
  • 82
  • 96
Babubabu
  • 69
  • 4

2 Answers2

7

This seems like a bug in FPC. To narrow down the problem it often helps to eliminate code and try to create a minimal example. This, for example, demonstrates the problem :

program project1;    
uses
  Classes, Crt;    
type
  TSortThread = class(TThread)
    protected
      procedure Execute; override;
    public
      constructor Create;
  end;

constructor TSortThread.Create;
begin
  inherited Create(False);
  FreeOnTerminate := False;
end;

procedure TSortThread.Execute;
begin
end;

var
  SortThread :  TSortThread;
begin
  Write('test ...');
  SortThread := TSortThread.Create;
  Writeln('created');
  SortThread.WaitFor;
  SortThread.Free;
  Writeln('complete');
  Readln;
end.

and produces output:

enter image description here

This seems like a bug in the console output only. Your original program, although it could certainly be improved in a sizeable number of ways, otherwise seems to sort the matrices correctly. This type of bug nevertheless does not inspire confidence in the FPC...

J...
  • 30,968
  • 6
  • 66
  • 143
  • 1
    What version of FPC are you using? I tested your code here with trunk (2.7.1) and [this](http://i.imgur.com/W2Ooipt.png) was the result. It might not inspire confidence for you but in my experience bugs like this are fixed really quickly when found (unlike in Delphi). – Rik Sep 21 '14 at 12:37
  • @Rik Using Lazarus V1.2.4, FPC V2.6.4 - default compile options, Win7-x64. This release is only a few months old. – J... Sep 21 '14 at 12:40
  • i used lazarus 1.0.12 when bug occured so i updated lazarus to 1.2.4 but bug still exists. – Babubabu Sep 21 '14 at 12:47
  • @Babubabu What counts is the FPC version – David Heffernan Sep 21 '14 at 12:49
  • 7
    The problem is Crt unit; after removing it from `uses` clause the code works as it should. FPC 2.6.2. – kludg Sep 21 '14 at 12:50
  • @user246408 Nice. Can't believe I missed cutting that out. Definitely corrects the problem. Should be an answer. – J... Sep 21 '14 at 13:14
  • Flushing output before thread creation might also be a workaround – Marco van de Voort Oct 09 '14 at 09:40
2

@user246408 Yes u re right the problem is CRT unit. i removed it from uses section and code started to work correctly.

Babubabu
  • 69
  • 4
  • 5
    F.Y.I. The [documentation of CRT](http://freepascal.org/docs-html/rtl/crt/) also states `The CRT unit is not thread safe.`. It's also in [this bugtracker-entry](http://bugs.freepascal.org/view.php?id=11554)... but, yeah.. that's easy to find if you **know** the problem is with CRT ;) – Rik Sep 21 '14 at 13:19
  • @Rik A lot of things are not threadsafe. I think it's a pretty enormous leap to go from *"not threadsafe"* to *"causes catastrophic failure when simply included, even if not used, in a multithreaded application"*. For something that is a framework unit, I'd call that unambiguously a broken unit. – J... Sep 21 '14 at 19:54
  • Units like Crt (or my Console unit) have to patch the output routines. That is why "simply including" is enough to make it have an effect. – Rudy Velthuis Sep 21 '14 at 20:08
  • 1
    @J Actually `causes catastrophic failure` is a big word. The program **runs correctly**. It's just the CRT (Screen output) that's messed up. For example... if you added another writeln and readln at the end you'll notice the program did run correctly and waits at the readln. It just doesn't show the crt/screen correctly anymore. Hence the not threadsafe notice, but it's just the screen... It certainly doesn't cause a catastrophic program failure. – Rik Sep 21 '14 at 20:28
  • @Rik Still, this is not the normal definition of thread safety. This unit cannot even coexist in an application that uses more than one thread. This is much different from code that cannot be used by multiple threads or code that must only be used in the main thread. In this case, the mere presence of a completely isolated background thread causes code to fail in the main thread. By my reckoning, that is intolerably fragile and, essentially, broken code. – J... Sep 21 '14 at 21:38
  • 1
    @J `By my reckoning, that is intolerably fragile and, essentially, broken code.` You're absolutely right. That's why: `The crt unit is not supported in multithreaded environments. It only exists for backward compatibility with Turbo Pascal programs.`. (Although the new code in 2.7.1 does seem to withstand this test-code, i haven't tested it with really using crt-functions **in** multi-threads) – Rik Sep 22 '14 at 08:37