Cryptopp: Singleton::Ref() crashes on VS2012/VS2013

[MarcelRaad]

CryptoPP::Singleton::Ref() in misc.h assumes thread-safe local statics. MSVC 11 (2012) and 12 (2013) do not support them, so locking the static mutex sometimes crashes.

I was not sure if thread-safe initialization of local static variables should be included in CRYPTOPP_CXX11_SYNCHRONIZATION or if it was only meant for library features, so I've created an issue instad of a pull request.

[noloader]

Thanks @MarcelRaad.

CryptoPP::Singleton::Ref() in misc.h assumes thread-safe local statics. MSVC 11 (2012) and 12 (2013) do not support them, so locking the static mutex sometimes crashes.

Wow, I thought C++11 sorted that out. Microsoft's docs don't indicate non-conformance or unexpected behavior.

A quick question... How do you know it works for VS2015 and MSC 1900?

I was not sure if thread-safe initialization of local static variables should be included in CRYPTOPP_CXX11_SYNCHRONIZATION

It sounds like we need to defer setting CRYPTOPP_CXX11_SYNCHRONIZATION until VS2015 and MSC 1900. Its controlled in config.h : 860. Do you feel like making a PR? (Also, put a comment in referring to the bug report so others can find it easily).

Should the same change be made for CRYPTOPP_CXX11_ATOMICS? I think the question is, will a common use case or pattern break or perform unexpectedly? If the answer is yes, then make the change. It looks like a yes to me:

#if defined(CRYPTOPP_CXX11_ATOMICS) && defined(CRYPTOPP_CXX11_SYNCHRONIZATION)
template <class T, class F, int instance>
  const T & Singleton<T, F, instance>::Ref(CRYPTOPP_NOINLINE_DOTDOTDOT) const
{
    static std::mutex s_mutex;
    static std::atomic<T*> s_pObject;
    ...
}

[noloader]

@MarcelRaad, @denisbider,

Bringing in DB in case he has some input. Denis, are you aware of any errata with static locals and C++11 when using C++11 primitives? C++11 remediated these problems, so I'm having trouble understanding how Microsoft states they provide the C++11 features but the program crashes.

I found these on Microsoft Connect:

1. runtime crash with static std::mutex.
2. Visual C++ 2015 runtime broken on Windows Server 2003 (C++11 magic statics)

(1) looks like the same old problem Microsoft has had since the late 1990s and early 2000s. They state they fixed it with magic statics. Unfortunately, (2) says the magic statics don't work and Microsoft won't be fixing them.

[MarcelRaad]

@noloader

Wow, I thought C++11 sorted that out. Microsoft's docs don't indicate non-conformance or unexpected behavior.

A quick question... How do you know it works for VS2015 and MSC 1900?

This is the "Magic statics" listed in the Concurrency section. In the VS2015 version of this document, they're listed as "Yes" for VS2015 and "No" for older versions:
https://msdn.microsoft.com/en-us/library/hh567368(v=vs.140).aspx

It sounds like we need to defer setting CRYPTOPP_CXX11_SYNCHRONIZATION until VS2015 and MSC 1900. Its controlled in config.h : 860 . Do you feel like making a PR? (Also, put a comment in referring to the bug report so others can find it easily).

Should the same change be made for CRYPTOPP_CXX11_ATOMICS ? I think the question is, will a common use case or pattern break or perform unexpectedly? If the answer is yes, then make the change. It looks like a yes to me:

I'll have a closer look.

[MarcelRaad]

@noloader

Unfortunately, (2) says the magic statics don't work and Microsoft won't be fixing them.

Wow! Then maybe there could even be a separate feature define for code relying on them?

The bug report says:

Windows Server 2003 and Windows XP have problems with dynamically loading a DLL (via LoadLibrary) that uses thread-local storage, which is what thread-safe statics use internally to provide efficient execution when the static local has already been initialized

Unfortunately I haven't found a macro that set when /Zc:threadSafeInit- is used, so the condition could be something like:
(_MSC_VER >= 1900) && ((_WIN32_WINNT >= 0x600) || !defined(_WINDLL))

[denisbider]

@noloader: The fundamental issue you are facing is that you have wider compatibility requirements than Microsoft provides for you.

Visual Studio 2015 provides thread-safe statics on versions of Windows supported by Microsoft. Windows XP and Windows Server 2003 are no longer supported by Microsoft.

Earlier Visual Studio versions do not provide thread-safe statics. At all.

If you want to:

• Support Windows XP and Windows Server 2003 using any version of Visual Studio

• Support older versions of Visual Studio

... then you should not be relying on thread-safe statics.

[noloader]

@MarcelRaad, @aj3423,

It really saddens me this is still a problem, even with C++11. Sorry about the situation. I think we have two choices:

1. Microsoft platforms use C++03 Singleton by default, and users must do something special to get C++11 Singleton.
2. Microsoft platforms use C++11 Singleton by default, and users must do something special to get C++03 Singleton.

The "it just works" option is (1) because the C++03 strategy works for both C++03 and C++11. The worst case is concurrent initialization, in which case there's a memory leak from the Singleton and a memory leak from the concurrent thread.

The "it usually works" option is (2) but its causing more grief than expected. The worst case is a crash for some users. The typical case is a single memory leak from the Singleton.

We are currently using the strategy from (2), but as issues 372 and 373 show, there are more breaks than expected. I know folks who are still using XP and Server 2003 in Automotive, Financial and Medical verticals, so more user problems are probably going to surface.

I think we need to switch to strategy (1) to protect all Microsoft users, and not just some of them. I despise asking a user who is running a modern Windows with modern Visual Studio to do something special, but I don't know how to reconcile things.

What do you guys think?

---

Another option is to remove the Singleton. Arguments about pattern vs anti-pattern aside, its easier said than done because I believe the Singleton is helping keep Static Initialization Order Fiasco in check.

[MarcelRaad]

@noloader Sounds good.
Maybe there could be a conservative default for using the C++11 version of Singleton even on MSVC 2015+ as it does work on Windows Vista+? When not using the XP toolset ((_MSC_VER >= 1900) && !defined(_USING_V110_SDK71_)) it's not possible to run the application on Windows XP / Server 2003 anyway.

[noloader]

I believe we cleared this issue at Commit 46c9cc725cf325f2.

The commit added CRYPTOPP_CXX11_DYNAMIC_INIT to pickup Dynamic Initialization and Destruction with Concurrency conformance. We use the C++11 version if CRYPTOPP_CXX11_ATOMICS, CRYPTOPP_CXX11_SYNCHRONIZATION and CRYPTOPP_CXX11_DYNAMIC_INIT are defined. Microsoft platforms have an additional restriction of Windows Vista or above.

Here are the related bug reports for the commit:

• Issue 372, Singleton::Ref() crashes on MSVC11/12
• Issue 373, Loader lock on WinXP
• Issue 389, Race condition in InitializeInteger::InitializeInteger
• Issue 391, Crash in NonblockingRng::GenerateBlock

[MarcelRaad]

Yes, perfect, thanks!

[Taulie79]

Hello,

am sorry, but the problem is not solved.
I use VS2015. link cryptlib.lib into a dll. the dll is loaded by embarcadero.

i got the described crashes in my exe.

my solution was to comment following line:
from: # define CRYPTOPP_CXX11 1
to: //# define CRYPTOPP_CXX11 1

[MarcelRaad]

@Taulie79 Are you sure that you're on current master, using the v140 toolset and not using /Zc:threadSafeInit-? I've never had that crash with MSVC 14.

[Taulie79]

v140 toolset - yes
/Zc:threadSafeInit - not exist
current master - is not released, i didn't tried it yet - i use (5.6.4 Sept/11/2016)
i see, i need an update if you release the new version