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

Problem in dump() in json.h caused by ss.imbue #444

Closed
cvengelen opened this issue Feb 3, 2017 · 7 comments
Closed

Problem in dump() in json.h caused by ss.imbue #444

cvengelen opened this issue Feb 3, 2017 · 7 comments

Comments

@cvengelen
Copy link

cvengelen commented Feb 3, 2017

We have been using json.h for Json input and output in our product without problems.

However, until now I have build a Debug version of the product, and now I started building for a Release configuration. I immediately got a Heap exception in the last statement of the dump() method of json. This must have been a problem in some destructor, so just to be sure I changed the dump code in the following way:

string_t dump(const int indent = -1) const
{
string_t tmp;
{
    std::stringstream ss;
    // fix locale problems
    ss.imbue(std::locale(std::locale(), new DecimalSeparator));

    if (indent >= 0)
    {
        dump(ss, true, static_cast<unsigned int>(indent));
    }
    else
    {
        dump(ss, false, 0);
    }

    // CvE Test heap problem for Release configuration
    tmp = ss.str();
}
    return tmp;
}

The Heap exception then occurs when the block statement with the ss declaration is left, so caused by the ss destructor. Also see attached VS2015 screenshot.

Just as a guess, I then commented out the ss.imbue call, and then the Heap exception disappeared immediately.

Again, note that this problem only occurs when compiling for the Release configuration (with /MT in VS2015 instead of /MTd).

jsonimbueproblem

@nlohmann
Copy link
Owner

nlohmann commented Feb 3, 2017

Hey @cvengelen, thanks for reporting. This looks really strange, because we are running the AppVeyor tests (see https://github.com/nlohmann/json/blob/develop/appveyor.yml) in Release mode.

@nlohmann
Copy link
Owner

nlohmann commented Feb 3, 2017

Maybe someone with more insights to MSVC can help.

@nlohmann nlohmann added the state: help needed the issue needs help to proceed label Feb 3, 2017
@TurpentineDistillery
Copy link

This is possibly #359.
Please try a newer version of the library.

@nlohmann
Copy link
Owner

nlohmann commented Feb 5, 2017

@cvengelen Which version of the library are you using?

@cvengelen
Copy link
Author

The project is using version 2.0.1. Note that there are no statics inthe call to ss.imbue:

	std::stringstream ss;
	// fix locale problems
	ss.imbue(std::locale(std::locale(), new DecimalSeparator));

I'll check with the latest json version.,

@cvengelen
Copy link
Author

This issue is resolved in version 2.1.0 (or in another version between 2.0.1 and 2.1.0).
In version 2.1.0, the call to ss.imbue is replaced by two separate calls:

    // fix locale problems
    ss.imbue(std::locale::classic());

    // 6, 15 or 16 digits of precision allows round-trip IEEE 754
    // string->float->string, string->double->string or string->long
    // double->string; to be safe, we read this value from
    // std::numeric_limits<number_float_t>::digits10
    ss.precision(std::numeric_limits<double>::digits10);

Somehow this solves my original problem: the heap exception does not occur anymore.
Thanks for the support!
Chris.

@nlohmann nlohmann removed the state: help needed the issue needs help to proceed label Feb 6, 2017
@nlohmann
Copy link
Owner

nlohmann commented Feb 6, 2017

Thanks for the feedback!

@nlohmann nlohmann closed this as completed Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants