Remove use of thread local storage to prevent artifacts on ARM platform

LibVNC / libvncserver

LibVNCServer/LibVNCClient are cross-platform C libraries that allow you to easily implement VNC server or client functionality in your program.

GNU General Public License v2.0

1.11k stars 484 forks source link

Remove use of thread local storage to prevent artifacts on ARM platform #490

Closed maxnet closed 2 years ago

maxnet commented 2 years ago

Removes the use of TLS variables in zlib and tightvnc encodings
Solves 2 memory leaks in which buffers were not being freed after client disconnects. The previous methods to free buffers had no chance of working because they were operating on per screen instead of per client, and being executed in the wrong thread. (Cannot see pointer to buffer in other thread if it is a TLS variable).
Prevents artifacts on ARM platform when using Tight encoding. (Probably caused by compiler optimization bugs, but debugging those is more work than getting rid of TLS).

bk138 commented 2 years ago

Thanks a lot! Will take some time to review, so please be patient!

JPDelprat commented 2 years ago

Thanks a lot! Will take some time to review, so please be patient!

Ref: @maxnet

Hello,@bk138

You classified this issue as "enhancement". In fact, it is also a bug since there are memory leaks (buffers are created on different threads, and rfbTightCleanup has no effect on them, since it is only called on one thread, in which buffers have not been created anyway). Note sure thread variables is a good idea ;-)

I really need this fix for my application. Do you know how long it will take to integrate it in master branch ?

Thanks

maxnet commented 2 years ago

I really need this fix for my application. Do you know how long it will take to integrate it in master branch ?

If you need something fast, you can also look if there is a 64-bit aarch64 Linux distribution for your board. Noticed the compiler bug only seems present on 32-bit arm.

bk138 commented 2 years ago

Thanks a lot! Will take some time to review, so please be patient!

Ref: @maxnet

Hello,@bk138

You classified this issue as "enhancement". In fact, it is also a bug since there are memory leaks (buffers are created on different threads, and rfbTightCleanup has no effect on them, since it is only called on one thread, in which buffers have not been created anyway). Note sure thread variables is a good idea ;-)

I really need this fix for my application. Do you know how long it will take to integrate it in master branch ?

Thanks

I'm currently overwhelmed by family and $$$-work. This will get done, please stay patient.

JPDelprat commented 2 years ago

I really need this fix for my application. Do you know how long it will take to integrate it in master branch ?

If you need something fast, you can also look if there is a 64-bit aarch64 Linux distribution for your board. Noticed the compiler bug only seems present on 32-bit arm.

Thanks for your answer. In fact, I am not concerned by the compiler bug, since I use a 64-bit architecture (android). My problem is the memory leak (tightvnc thread variable buffers not being freed), because I create/delete the vnc server many times, and my client also frequently connect/disconnects.

JPDelprat commented 2 years ago

Thanks a lot! Will take some time to review, so please be patient!

Ref: @maxnet Hello,@bk138 You classified this issue as "enhancement". In fact, it is also a bug since there are memory leaks (buffers are created on different threads, and rfbTightCleanup has no effect on them, since it is only called on one thread, in which buffers have not been created anyway). Note sure thread variables is a good idea ;-) I really need this fix for my application. Do you know how long it will take to integrate it in master branch ? Thanks

I'm currently overwhelmed by family and $$$-work. This will get done, please stay patient.

Thanks for your answer. I understand ;-) Meanwhile, I will use @maxnet fork. Thanks a lot.