Find a problem on "multiplayer"

[Phoenix]

Hello
Even without the bug that I solved today , the "multiplayer" example does not work yet.

If I run it in debug mode it hangs on:

Tuos_Player.execute
...
for x3 := 0 to high(StreamIn[x2].Data.Buffer) do
  begin
  StreamOut[x].Data.Buffer[x3] :=
  cfloat(StreamOut[x].Data.Buffer[x3]) + <-- 'RunError(201)'
  cfloat(StreamIn[x2].Data.Buffer[x3]);
  end;  
...

but

if I change the lines

"uos_AddIntoDevOut(PlayerIndex3, -1, -1, -1, -1, 0, 1024);"

with

"uos_AddIntoDevOut(PlayerIndex3, -1, -1, -1, -1, 0, -1);"

work!

at form closure, memory leaks occur:

it simply lacks

BufferBMP.free;

 for the logo

Windows 10 64bit
Laz 1.8.0 64bit - FPC 3.0.4


 Thanks for your job!!

[fredvs]

Hello Phoenix.

Thanks to report this.

I will, asap, test it with Windows, I never found problem with multiplayer with unix os.

Write you later.

Fre;D

[fredvs]

Hello Phoenix.

Well seen !

Yes, there was a (big) problem with MultiPlayer demo.

Huh, it is my fault, I do not use LCL widgetset for a long time now and did not test all the LCL demos with new uos code.

It is fixed now (commit 4ded849).

In the future, I will always test the LCL demos too.

Thanks to note it and sorry for the inconvenient.

Fre;D.

[Phoenix]

Hello, Sorry for the delay to reply.
Thank you for making these changes! Now all ok.
At the moment I'm checking the uos.pas and uos_flat.pas for find another problems (maybe I'll do another pull request or write here) .

[Phoenix]

Hello,
I just created the pull request that solves various problems.

[fredvs]

Hello Phoenix.

Yep, your changes are committed.

Many thanks for your great contributions.

Fre;D

[Phoenix]

Hello,
I'am happy!

Sorry I was not quick enough to add the last two commits in time .
So I had to create a new pull request .
At the moment I have not seen other things.

[fredvs]

Re-hello Phoenix.

Huh, I get memory leak with uos_Free() in uos_flat.pas.

I did commit code that fix it  ---> commit 5a10b37.

Maybe there is something that I did not catch but now all seems ok, even for console or LCL or fpGUI or MSE...

Fre;D

[Phoenix]

Hello,
at the moment I have no memory leak (but you've certainly tested more different cases).

Give me some details about the problem or source for test.

[Phoenix]

I watched the changes you've restored.

+  if assigned(uosPlayers) then  (1)
    if length(uosPlayers) > 0 then
     for x := 0 to length(uosPlayers) -1 do
+    begin
       if assigned(uosPlayers[x]) then
-       uosPlayers[x].FreePlayer;

+    begin
+      uosPlayers[x].nofree := false; (2)
+      uos_stop(x); (3)
+      uos_freeplayer(x); (4)
+    end;
+  end;

(1) it's useless (it is an initialized dynamic array)
(2) assigned up in the FreePlayer method
(3) call in the FreePlayer method
(4) call FreePlayer

procedure Tuos_Player.FreePlayer();  // Works only when PlayNoFree() was used: free the player
begin
  if isAssigned then
  begin
    NLooped := 0;
    NoFree := False;
    //if it has never been put into play (= there is no thread for free)..
    if thethread = nil then
      Play();
    Stop;
  end;
end;   

The differences are few. Perhaps in your test you have a Tuos_Player never executed?

[fredvs]

Hello Phoenix.

The memory leak appears when using MSE-threads.

With the last committed code memory leak disappear.

I will investigate more to understand why.

(Maybe a {$IF DEFINED(mse)} is needed in uos_Free() of uos_flat.pas when MSE-threads are used).

Fre;D

[fredvs]

> Give me some details about the problem or source for test.

In StrumPract ---> https://github.com/fredvs/strumpract

Fre;D

[Phoenix]

Hello,
yesterday evening I was too late, I had some problems with MSEide
In the end I saw the problem: the player's memory is not freed.

Now I understand why with the change goes:

procedure uos_Stop(PlayerIndex: cint32);  // Stop playing and if uos_Play() was used: free the player
begin
  if (length(uosPlayers) > 0) and (PlayerIndex +1 <= length(uosPlayers)) then
  if  uosPlayersStat[PlayerIndex] = 1 then
   if assigned(uosPlayers[PlayerIndex]) then
   begin
uosPlayers[PlayerIndex].Stop();
{$IF DEFINED(mse)}  <----------------------
  freeandnil(uosPlayers[PlayerIndex]);
  uosPlayersStat[PlayerIndex] := -1 ;
{$endif}
end;
end; 

I've never used these libraries.
But if you put FreeOnTerminate True and an OnTerminate .. Practically like for the thread in lazarus.

In any case freeandnil at Stop () is not ok: if you stop () on a Player with NoFree?

[Phoenix]

1) remove (this block is an error)

procedure uos_Stop(PlayerIndex: cint32);  
begin
  ...
{$IF DEFINED(mse)} <-------------
  freeandnil(uosPlayers[PlayerIndex]);
  uosPlayersStat[PlayerIndex] := -1 ;
{$endif}
end;
end;

2) in uos.pas replace (it is safer to FreeAndNil () or at least Free ())
NOTE: I think that in other parts of uos the ".destroy" should be replaced with ".free"!!

{$IF DEFINED(mse)}
  theplayer.destroy;
  {$endif}

with

{$IF DEFINED(mse)}
  FreeAndNil(theplayer);
  {$endif}

3) For the FreeOnTerminate just add a "True" here

...
{$IF DEFINED(mse)}
  thethread:= tmsethread.create(@execute,True);
  {$else}  
...

4) ??? I do not see OnTerminate or similar in this mse class

Looking for I found the tthreadcomp class.
so it should be replaced tmsethread with this that has onterminate and use as "options" "[tco_autorelease]" for FreeOnTerminate I think (I have not tested).

Or now I have an idea that I do not know if it works but if you use the destructor of tmsethread to create an "OnTerminate"?

[Phoenix]

1) + 2) + 3) +

4a)

...
type 
{$IF DEFINED(mse)}
 TuosThreadMse = Class(tmsethread)
  public
   theparent : Tobject;
   procedure DoTerminate; virtual;
   destructor Destroy; override;
 end;
{$else}
 TuosThread = Class(TThread)
  protected
   ....

4b)

...
 protected
   {$IF DEFINED(mse)}
   thethread : TuosThreadMse; 
   {$else}
   thethread : TuosThread;
   {$endif}   
 ...

4c)

...
{$IF DEFINED(mse)}
destructor TuosThreadMse.Destroy;
begin
  DoTerminate;
  inherited;
end;
{$else}
constructor TuosThread.Create(CreateSuspended: boolean; AParent: TObject;
  const StackSize: SizeUInt);
... 

4d)

...
{$IF DEFINED(mse)}
  thethread:= tuosthreadmse.create(@execute,True);
  thethread.theparent:= Self;
  {$else}
  thethread:= tuosthread.Create(false,self);
  {$endif}
...

4e)

...
{$IF DEFINED(mse)}
procedure TuosThreadMse.DoTerminate;
begin
 //notice that is no longer valid (for safe destroy event of theparent)
 Tuos_Player(theparent).thethread:= nil;
 //execute player destroy
 FreeAndNil(theparent);
end;
{$else}
procedure TuosThread.DoTerminate;
begin       
...

4f) Delete this block (if thread not closed = never call Player destroy)

...
destructor Tuos_Player.Destroy;
var
  x: cint32;
begin

if thethread <> nil then begin <----------------------------
   {$ifdef mse} 
    thethread.terminate();
   application.waitforthread(thethread);
    //calls unlockall()/relockall in order to avoid possible deadlock
    thethread.destroy();
  {$endif}
    
  end;
...

Ok, I think the idea is good!

Can you test it?

[fredvs]

> But if you put FreeOnTerminate True and an OnTerminate

FreeOnTerminate does not exist with MSE-threads.

Fre;D

[fredvs]

Hello Phoenix.

Wow, lot of good ideas.

I will study it deeply.

By the way, after many test, including closing application while some PlayerNoFree are still playing, this is needed in uos_free():

if length(uosPlayers) > 0 then
 for x := 0 to length(uosPlayers) -1 do
  begin
  uosPlayers[x].nofree := false;
  uos_stop(x);
  end;

Now I cannot find any memory leak with both MSE-threads and FPC-threads.

It was changed in commit 8e46ad1

Fre;D

[Phoenix]

Hello,

fredvs wrote

FreeOnTerminate does not exist with MSE-threads.

thethread:= tuosthreadmse.create(@execute,True);

There is: TRUE represents that

[Phoenix]

Ok, in case if I find any problem I write here.
Thank you!

[fredvs]

> thethread:= tuosthreadmse.create(@execute,True);

Ooops ---> tmsethread = class
...
 constructor create(const athreadproc: threadprocty;
                const afreeonterminate: boolean = false;
                const astacksizekb: integer = 0); overload; virtual;
...

Indeed. ( IMO, Martin should give more infos in his examples (and me look more at source ).

Well seen, I will study-test-commit this asap.

Many thanks.

Fre;D