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

fix pd_this on Windows #2165

Merged
merged 1 commit into from
Mar 14, 2024
Merged

Conversation

Spacechild1
Copy link
Contributor

@Spacechild1 Spacechild1 commented Jan 4, 2024

On Windows it is not possible to 'properly' export a thread local variable from a DLL. MinGW can do it, but the resulting symbol (__emutls_v.pd_this) is not portable and certainly not understood by MSVC. As a consequence, Windows externals built with -DPDINSTANCE cannot really use pd_this - unless both the external and the host is built with MinGW.

The solution is to access pd_this via a function call, for this I've added a new API function pd_getinstance(). For externals, pd_this is now a macro for pd_getinstance(). Internally, we directly access 'pd_this', but we don't try to export it.

On other platforms, pd_this is now properly marked EXTERN.

@Spacechild1 Spacechild1 added bug/fix either a bug (for issues) or a bugfix (for pull-requests) subject:API "External Programming Interface" (things concerning development of externals) labels Jan 4, 2024
@Spacechild1
Copy link
Contributor Author

Just out of curiosity, I quickly checked on Linux: it does indeed export a symbol pd_this, but this is obviously not a pointer to the (thread-local) variable itself; rather, this is the index of the variable in the dynamic thread vector. When an external accesses pd_this, its value is automatically passed to the __tls_get_addr function, which returns the actual address of the variable for this particular thread. Now, __tls_get_addr comes from glibc and is not a standard C function. If we want our API to be a pure standard C API, we should probably apply the Windows solution to all other platforms. On the other hand, the current mechanism on Linux and macOS seems to work fine and is unlikely to change in the future (in particular, the meaning of the pd_this value).

What do you (@umlaeute, @danomatika and @millerpuckette, in particular) think about this?

(If anyone is interested in how thread-local storage actually works in Linux, I found this blog post very helpful: http://david-grs.github.io/tls_performance_overhead_cost_linux/).

Copy link
Contributor

@umlaeute umlaeute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good (after dropping the EXTERN decorator from the C-file)

src/m_class.c Outdated Show resolved Hide resolved
On Windows it is not possible to 'properly' export a thread local variable from a DLL.
MinGW can do it, but the resulting symbol ('__emutls_v.pd_this') is not portable and certainly not understood by MSVC.

The solution is to access 'pd_this' via a function call.
For externals, 'pd_this' is now a macro for 'pd_getinstance()'.
Internally, we directly use 'pd_this', but we don't try to export it.

On other platforms, 'pd_this' is now properly marked as EXTERN.
@Spacechild1
Copy link
Contributor Author

after dropping the EXTERN decorator from the C-file)

Fixed!

@Spacechild1 Spacechild1 changed the title fix 'pd_this' on Windows fix pd_this on Windows Jan 6, 2024
@danomatika
Copy link
Contributor

@millerpuckette This is needed for libpd.

@millerpuckette millerpuckette merged commit 54f42a9 into pure-data:master Mar 14, 2024
9 checks passed
@Spacechild1 Spacechild1 deleted the pd_this_fix branch May 9, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/fix either a bug (for issues) or a bugfix (for pull-requests) subject:API "External Programming Interface" (things concerning development of externals)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants