Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement an abstraction over pthread and windows thread synchronisation primitives #1002

Closed
iphydf opened this issue Jul 14, 2018 · 4 comments
Labels
P3 Low priority wontfix Won't Fix (Intended behaviour)
Milestone

Comments

@iphydf
Copy link
Member

iphydf commented Jul 14, 2018

It should use pthread for posix systems, windows threads for windows systems. We should get rid of the dependency on pthread-win32 and instead provide small wrappers around OS-specific code ourselves. This makes compiling with msvc easier.

@nurupo
Copy link
Member

nurupo commented Jul 14, 2018

Implement an abstraction over pthread and windows thread synchronisation primitives

So, "Implement pthreads-win32".

Someone already has done that already though, here https://www.sourceware.org/pthreads-win32/.

@zoff99
Copy link

zoff99 commented Jul 14, 2018

last release was in 2099 :-)

RELEASE 2.10.0
--------------
(2099-99-99)

@iphydf iphydf added this to the v0.2.x milestone Jul 16, 2018
@iphydf
Copy link
Member Author

iphydf commented Jul 19, 2018

  1. Having to #include <pthread.h> means all the "portable" C code is now tainted with OS-specific includes, making static analysis a lot harder.
  2. We don't actually need most of pthreads-win32, we only need mutexes, so making a small wrapper that exposes exactly what we need allows us to strictly limit the amount of system-dependent APIs we use. So no, this is not "implement pthreads-win32".

@nurupo
Copy link
Member

nurupo commented Jul 19, 2018

  1. If all you want is not to bring OS-specific includes in the scope, then just hide them, make a tox_pthread.h and tox_pthread.c wrapper around pthread.h that will hide them inside tox_pthread.c.
  2. Making such wrapper is easy and will also allow us to use all of the other features of pthreads-win32 besides mutexes if a need arises, because otherwise, if we would need more stuff than mutexes, we'd need to either implement the missing OS abstraction ourselves or scrap all the mutex code you are proposing to write and implement this pthread.h wrapper I'm suggesting.

pthread.h wrapper hiding the OS-specific includes is easy to make, requires no maintenance, pthreads-win32 on Windows is already implemented and battle-tested so there are unlikely to be bugs, and it's easily extendable.

@iphydf iphydf closed this as completed Jul 20, 2018
@iphydf iphydf added the wontfix Won't Fix (Intended behaviour) label Jul 20, 2018
@iphydf iphydf modified the milestones: v0.2.x, v0.2.5 Aug 4, 2018
@iphydf iphydf added the P3 Low priority label Feb 4, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Low priority wontfix Won't Fix (Intended behaviour)
Projects
None yet
Development

No branches or pull requests

3 participants