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

Add built-in dynamic build with static CRT #19005

Closed
wants to merge 2 commits into from

Conversation

NN---
Copy link
Contributor

@NN--- NN--- commented Jul 19, 2021

Describe the pull request

Adding new triplet for Windows.
It is possible to add it manually, however I see many developers are struggling with the same issue.
Building dynamic library (DLL) while not using dynamic dependency on VC Runtime.

@NN--- NN--- force-pushed the dynamic_with_static_crt branch from 7068a7b to 655f42a Compare July 19, 2021 11:43
@Neumann-A
Copy link
Contributor

I think the answer to this was a big NO, since it can introduce serious bug if objects get passed over DLL boundaries and allocated and then deallocated by different CRTs.

@NN---
Copy link
Contributor Author

NN--- commented Jul 19, 2021

This is known issue. If you pass C++ objects over dynamic library boundary without using the same CRT it is not safe.
This why dynamic libraries expose C API and not C++ API.
Check the issues, many people need this triplet.

@Neumann-A
Copy link
Contributor

This why dynamic libraries expose C API and not C++ API.

This is not generally true.

Check the issues, many people need this triplet.

--overlay-triplets=<path_to_your_dangerous_triplets> ?

@NN---
Copy link
Contributor Author

NN--- commented Jul 19, 2021

Whoever needs this triplet, knows why it is needed.
I just ask to have it bundled with vcpkg.

@JackBoosY JackBoosY added the category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. label Jul 20, 2021
Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Since they are not the official triplets, please move these triplet file to triplets/community .

@NN---
Copy link
Contributor Author

NN--- commented Jul 20, 2021

@JackBoos Done.

@NN--- NN--- force-pushed the dynamic_with_static_crt branch from ac1a3f1 to e270bd2 Compare July 20, 2021 06:45
Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

How about rename the triplets to x64-windows-mt and x86-windows-mt?

@NN---
Copy link
Contributor Author

NN--- commented Jul 20, 2021

Do you have a better name ?

@JackBoosY
Copy link
Contributor

@NN--- I think x64-windows-mt and x86-windows-mt would be okay.

@NN--- NN--- requested a review from JackBoosY July 21, 2021 10:07
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jul 22, 2021
@JackBoosY JackBoosY changed the title Add built-in dynamic build with static CRT. Add built-in dynamic build with static CRT Jul 22, 2021
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

I don't believe we can ever merge this.

Most libraries in vcpkg are not prepared to deal with this type of configuration. They speak standard library types in their interfaces, or throw exceptions, neither of which are safe to do when the standard library types are "owned" by different C runtime instances. When you statically link with the C runtime in a DLL, each DLL gets its own entire copy of the C runtime, and all state that may depend on global variables therein is unsafe to use.

For example, the iterator debug machinery is protected by the debug lock, which is a global variable that lives in msvcp140.dll/libcpmt.lib. If you have a DLL that exposes a function that has a const std::vector&, that library cannot be used in such a configuration, because each DLL thinks it is taking a lock protecting the debugging data structures, when in reality they are taking different locks.

(This problem is similar to static libraries using a dynamic CRT as previously discussed in #16387 and #15122 , but the probability of triggering the problem goes from "possible" to "certain" in this case, as even use of the core language triggers breakage.)

@BillyONeal
Copy link
Member

I also observe that this change seems unrelated to #36 (which requests completely static linking served by today's foo-windows-static triplets) and #15386 (which again is served by complete static linking)

@JackBoosY JackBoosY removed the info:reviewed Pull Request changes follow basic guidelines label Jul 23, 2021
@NN---
Copy link
Contributor Author

NN--- commented Jul 23, 2021

This is correct that using C++ API between libraries in such scenario is problematic.
However there is no problem when you use a library with C API.
For instance libssh exposes C API, therefore you can use it safely.

@ras0219-msft
Copy link
Contributor

@BillyONeal's comment #19005 (review) explains why this can never be an official or community vcpkg triplet.

However, if you are an advanced user with one of the few valid use cases, this is always a possible triplet to define on your own and use via overlay-triplets.

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for dynamic runtime with static linking?
5 participants