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

Windows (Visual Studio) use Multi-threaded Debug/Release #208

Closed
bromix opened this issue Mar 12, 2020 · 5 comments
Closed

Windows (Visual Studio) use Multi-threaded Debug/Release #208

bromix opened this issue Mar 12, 2020 · 5 comments

Comments

@bromix
Copy link

bromix commented Mar 12, 2020

I encountered a strange problem while using ifstream or stringstream. I'm implementing a function to read settings for an application from an ini file and return the settings as an object.
But as soon as the stringstream is declared, I get a heap corruption when leaving the scope of the function. Has this to do with threading and if so, how can I secure this? The documentation is not clear for me how to accomplish this. Below is a simplified implementation of my problem.

I'm working with Node 13.10.1

The solution to this problem is, to set the project to Multi-threaded Debug/Release (not DLL)

#include <sstream>

Napi::Value load_from_ini_string(const Napi::CallbackInfo& info)
{
	const Napi::Env env = info.Env();
	if(info.Length() != 1)
	{
		Napi::TypeError::New(env, "Wrong number of arguments").ThrowAsJavaScriptException();
		return env.Null();
	}

	if (!info[0].IsString())
	{
		Napi::TypeError::New(env, "Wrong arguments").ThrowAsJavaScriptException();
		return env.Null();
	}

	auto ini_content = info[0].As<Napi::String>().Utf8Value();

	std::stringstream ss;  // <- this is the problem
	// as soon as this line exists and we leave the scope of this function,
	// we get a heap corruption.
	
	return Napi::String::New(env, "Hello");
}

void init(Napi::Env env, Napi::Object exports)
{
	exports.Set("load_from_ini_string", Napi::Function::New(env, load_from_ini_string));
}
@japj
Copy link

japj commented May 1, 2020

@bromix Can you try to put this in the top of your CMakeLists.txt? (I ran into the same issue)
Note this might require a fairly recent version of cmake (I am using cmake 3.17).

# workaround for cmake-js not setting MSVC multi-threaded statically-linked runtime library.
set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")

@bromix
Copy link
Author

bromix commented May 3, 2020

@japj See my comment ;)

The solution to this problem is, to set the project to Multi-threaded Debug/Release (not DLL)

This solution worked for me, so the setting also:
set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")

The runtime (Windows only) must be linked statically and not dynamically.

@AtiqGauri
Copy link

@japj Sadly this workaround is for windows only

@wsw364321644
Copy link

Is there any solution for /MD build .

@Julusian
Copy link
Collaborator

Julusian commented Oct 8, 2022

I believe this is crash is only a problem when nodejs and your module are built with different versions of MSVC, and your module is built as /MD. So I don't think there is a good solution other than to build as /MT.

In v7.0.0, cmake-js is injecting set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>") for you to try to make MT the default.
But I believe this relies on running cmake 3.15 or newer, and the file having cmake_policy(SET CMP0091 NEW) before the project definition.
The docs are updated to recommend this as a default, which is the best I can do to enforce this

@Julusian Julusian closed this as completed Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants