Synapse TCP/IP client and server

[Phoenix]

Please note that synapse use blocking sockets, with limited support for non-blocking mode. While you send a large file you cannot send a text in parallel, unless you spawn another worker thread both on the client end server side, just for the file transfer. I can make the client  more responsive(see attachment), but the transfer won't be faster.
You can even complicate things by sending a large file from client to server and in the same time another large file from server to client. It won't work in parallel either in the current implementation.

Yes, synchronicity is the strong point. The goal was just to send tasks when you want without blocking the button.

The task list is not locked, one ore more task can be overwritten or deleted. Even if you manage to copy the next task in a safe way, ProcessTask won't work faster, you need extra threads.

You're right, the lock in GetNextTask() is not enough because it must be maintained for the entire execution. Thank you 

[balazsszekely]

@Phoenix

You're right, the lock in GetNextTask() is not enough because it must be maintained for the entire execution.

Actually GetNextTask is safe, somehow I missed the function body. Sorry for that! What I said in my previous post still holds though, the client/server communication won't be faster.
Also I implemented a much better, non blocking way to add task to tasklist, Application.ProcessMessages is not neeeded. Search for function AddTaskInThread, I did not test it extensively, it might contain bugs/leaks. Please test it.

PS: If you wish to improve the code, I have a few more ideas(unfortunately almost no free time).

[Phoenix]

Hi @GetMem,

Actually GetNextTask is safe, somehow I missed the function body. Sorry for that!

It is OK. But even my answer shows that I don't know much about synapse  .. otherwise maybe I would have insisted more 

I did not test it extensively, it might contain bugs/leaks. Please test it.

thanks  , the code seems to work as expected. With the previous modifications a memory leak was introduced, now I fixed it (see attachment).

PS: If you wish to improve the code, I have a few more ideas(unfortunately almost no free time).

Ideas are always welcome, then for the implementation one always hopes not to be alone. I think that those who benefit from your demo can contribute in turn according to their possibilities.

[balazsszekely]

@Phoenix

Ideas are always welcome, then for the implementation one always hopes not to be alone. I think that those who benefit from your demo can contribute in turn accordingto their possibilities.

Ideas:
1. When accessing FThreadList(TThreadList) in TTCPBase class, the following mechanism is used:

Code: Pascal  [Select][+][-]

1.   List := FThreadList.LockList;
2.   try
3.     //..
4.   finally
5.     FThreadList.UnlockList;
6.   end;        

Which is fine, but not enough. With the current method, if more then one client is connecting at the same time, the server might miss a few clients. Another Lock/Unlock mechanism should be implemented, similar to the one in TTaskList, using CriticalSections and the FBlocked boolean. TThreadList can be replaced with a simple TList because it will be protected with CS.
This is a bug and should be fixed.
2. Create working threads for filetransfer, to make both client/server faster and more responsive. @piola already implemented something similar in reply #35
3. Fixing the daemon/service issue, @datilas post in reply #52
4. Switching from TCP to UDP when broadcasting
5. Implement UDP hole punching
6. Create a github/gitlab/bitbucket/sourforge(/ = or) repository. Currently we are ping-ponging with the source files, like we did in the '90. It' s so unprofessional and dirty, I almost enjoying it. Seriously though we need a repository. I have enough on my plate with OPM, it would be really nice if somebody could take care of the repository.

I attach the whole project again, with the same uTCPServer unit from your last post, minus a few comments("//#") , those are not needed anymore.

[Phoenix]

Currently we are ping-ponging with the source files, like we did in the '90. It' s so unprofessional and dirty, I almost enjoying it.

For now let's continue with the fun  , why stop  ? but I agree that a repository is needed.

1. When accessing FThreadList(TThreadList) in TTCPBase class, the following mechanism is used:

Code: Pascal  [Select][+][-]

1.   List := FThreadList.LockList;
2.   try
3.     //..
4.   finally
5.     FThreadList.UnlockList;
6.   end;        

Which is fine, but not enough. With the current method, if more then one client is connecting at the same time, the server might miss a few clients. Another Lock/Unlock mechanism should be implemented, similar to the one in TTaskList, using CriticalSections and the FBlocked boolean. TThreadList can be replaced with a simple TList because it will be protected with CS.
This is a bug and should be fixed.

I attach the code that hopefully solves but there are some doubts that I signaled with "#"

P.S.
I forgot I saw that in the classes many variables are not "initialized". I was wondering, if you use some compiler optimization options maybe they are no longer "set to zero"? I personally always initialize everything

[balazsszekely]

@Phoenix

Thanks for the modifications!

I attach the code that hopefully solves but there are some doubts that I signaled with "#"

Everything looks fine, except when two clients simultaneously disconnect and the threads are removed from the servers list. It looks to me that the first thread will lock the list and the second one will be ignored. The "If not FBlocked then ..." condition is missing. Unfortunately in this particular case it's very hard to check for FBlocked without slowing down the main thread. I still have an uncontrollable urge to rewrite the whole thing, it would be much easier this time, because I learned from my mistake.

I personally always initialize everything

Good practice! I tend to cheat and initialize only what is absolutely necessary. Most of the time I get away with it without any issues.

[Phoenix]

I still have an uncontrollable urge to rewrite the whole thing, it would be much easier this time, because I learned from my mistake.

If you already have clear ideas then the result will be better. Subsequent steps will probably make the code less maintainable because you need to constantly adapt. So feel free to rewrite when you find the time.

I tend to cheat and initialize only what is absolutely necessary. Most of the time I get away with it without any issues.

Nice to be so lucky  , unfortunately I have had opposite educational experiences  . However consider the idea in your rewrite so the code will be more robust.

Everything looks fine, except when two clients simultaneously disconnect and the threads are removed from the servers list. It looks to me that the first thread will lock the list and the second one will be ignored.

Among the ideas perhaps you should consider a unit test. Simulating the worst cases can go a long way in gaining free time  .

[balazsszekely]

Moved to: https://forum.lazarus.freepascal.org/index.php/topic,62874.msg475673.html#msg475673