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

Potential memory leak detected by Valgrind #1713

Closed
muttleyxd opened this issue Aug 20, 2019 · 4 comments
Closed

Potential memory leak detected by Valgrind #1713

muttleyxd opened this issue Aug 20, 2019 · 4 comments
Labels
kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated

Comments

@muttleyxd
Copy link

  • What is the issue you have?
    I encountered a weird memory leak in my code, when returning nlohmann::json object from my function, if function returning the object (of any type) throws then 32 bytes are leaked.
    This only happens on GCC, doesn't happen on Clang.

  • Please describe the steps to reproduce the issue. Can you provide a small but working code example?
    Code:
    json-valgrind.cpp

#include <nlohmann/json.hpp>

int throw_error()
{
  throw std::runtime_error("leak memory");
}

nlohmann::json minimal_leak()
{
  return {{"leak", throw_error()}};
}

int main()
{
  try {
    minimal_leak();
  } catch(...) { }
  return 0;
}

build:

g++ json-valgrind.cpp -o json-valgrind -g -O0

run:

valgrind ./json_valgrind
  • What is the expected behavior?
    Valgrind exits saying that definitely lost: 0 bytes in 0 blocks

  • And what is the actual behavior instead?
    Valgrind exits saying that definitely lost: 32 bytes in 1 blocks

  • Which compiler and operating system are you using? Is it a supported compiler?
    Tested on Manjaro Linux with:

    • GCC 9.1.0
    • GCC 8.3.0
    • Ubuntu 18.04 Docker container with GCC 7.4.0
  • Did you use a released version of the library or the version from the develop branch?
    Tested with release 3.7.0 and 3.5.0

I wonder if this is nlohmann/json bug or GCC bug, since it doesn't happen on Clang

@gregmarr
Copy link
Contributor

gregmarr commented Aug 29, 2019

I don't think this call even hits any code from this library. The difference between the versions can be explained by the fact that the order of parameter evaluation isn't guaranteed to be left to right. My guess here is that GCC creates a std::string for "leak" and then doesn't clean it up when the exception is thrown, and Clang executes the function call first before creating the std::string.

I suspect that you could reproduce this with this code that doesn't involve the json library:

int throw_error()
{
  throw std::runtime_error("leak memory");
}

std::pair<std::string, int> minimal_leak()
{
  return {{"leak", throw_error()}};
}

int main()
{
  try {
    minimal_leak();
  } catch(...) { }
  return 0;
}

@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Sep 2, 2019
@nlohmann
Copy link
Owner

@muttleyxd Did you have the chance to check #1713 (comment)?

@muttleyxd
Copy link
Author

@gregmarr @nlohmann
Sorry for my late reponse

This:

#include <stdexcept>
#include <string>
#include <utility>

int throw_error()
{
  throw std::runtime_error("leak memory");
}

std::pair<std::string, int> minimal_leak()
{
  return {"leak", throw_error()};
}

int main()
{
  try {
    minimal_leak();
  } catch(...) { }
  return 0;
}

exits fine, saying that 0 bytes were lost

I tried doing something like

std::pair<std::pair<std::string, int>, std::pair<std::string, int>> minimal_leak()
{
  return {{"leak", 123}, {"leak", throw_error()}};
}

but it won't leak anything.

@stale
Copy link

stale bot commented Oct 19, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Oct 19, 2019
@stale stale bot closed this as completed Oct 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated
Projects
None yet
Development

No branches or pull requests

3 participants