Crash when traversing over items() of temporary json objects

[BOT-Man-JL]

• What is the issue you have?

1. crash when traversing over items() of temporary json objects
2. sometimes got null of pair.value() (https://godbolt.org/z/TTQFHW)

• Please describe the steps to reproduce the issue. Can you provide a small but working code example?

  const auto obj = nlohmann::json::parse(R"({
    "key1": 0.1,
    "key2": 0.2
  })");

  auto rvalue = [&]() -> nlohmann::json { return obj; };
  for (const auto& pair : rvalue().items()) {
    std::cout << pair.key() << ": " << pair.value() << "\n";  // crash
  }

  auto crvalue = [&]() -> const nlohmann::json { return obj; };
  for (const auto& pair : crvalue().items()) {
    std::cout << pair.key() << ": " << pair.value() << "\n";  // crash
  }

• What is the expected behavior?

1. pair.value() returns 0.1 or 0.2
2. no crash

• And what is the actual behavior instead?

1. crash on my linux machine: json.hpp:14028: void nlohmann::detail::serializer<BasicJsonType>::dump(const BasicJsonType&, bool, bool, unsigned int, unsigned int) [with BasicJsonType = nlohmann::basic_json<>]: Assertion ‘false' failed.
2. pair.value() returns null on Compiler Explorer. (https://godbolt.org/z/TTQFHW)

• Which compiler and operating system are you using? Is it a supported compiler?

1. linux/centos
2. gcc 6.2

• Did you use a released version of the library or the version from the develop branch?

1. latest dev branch
2. release v3.7.3

[nlohmann]

When you create rvalue().items(), an iteration_proxy is created that captures the JSON object to iterate by reference. However, the object created by rvalue() is destroyed too soon, and when the iteration proxy is used, it has a reference to a destroyed object. When you use ASAN, you see the details.

I am not sure how to fix this.

[BOT-Man-JL]

However, it works fine when traversing over temporary json array:

  const auto arr = nlohmann::json::parse(R"([
    "key1",
    "key2"
  ])");

  std::cout << "not crash1:" << std::endl;
  auto rvalue = [&]() -> nlohmann::json { return arr; };
  for (const auto& item : rvalue()) {
    std::cout << item << "\n";
  }

  std::cout << "not crash2:" << std::endl;
  auto crvalue = [&]() -> const nlohmann::json { return arr; };
  for (const auto& item : crvalue()) {
    std::cout << item << "\n";
  }

https://godbolt.org/z/5Y-fxW

[BOT-Man-JL]

Could we temporarily disable rvalue().items() and crvalue().items() by overloading with reference qualifier &&?

iteration_proxy<iterator> items() & noexcept
{
    return iteration_proxy<iterator>(*this);
}

iteration_proxy<const_iterator> items() const & noexcept
{
    return iteration_proxy<const_iterator>(*this);
}

iteration_proxy<iterator> items() && noexcept
{
    static_assert(false_v<iterator>, "items() of rvalue is not supported.");
}

iteration_proxy<const_iterator> items() const && noexcept
{
    static_assert(false_v<iterator>, "items() of const rvalue is not supported.");
}

[ArtemSarmini]

Ref overloads are great, but such fix is a breaking change. Code like

template <typename R>
void iterate(R r);

iterate(get_json().items());

is valid. Though I hope there isn't much code like that.

[BOT-Man-JL]

I found this to be a undefined behavior

If range_expression returns a temporary, its lifetime is extended until the end of the loop, as indicated by binding to the forwarding reference __range, but beware that the lifetime of any temporary within range_expression is not extended.

for (auto& x : foo().items()) { /* .. */ } // undefined behavior if foo() returns by value

This problem may be worked around using init-statement (C++20):

for (T thing = foo(); auto& x : thing.items()) { /* ... */ } // OK

So, is there a way to work around in C++ 11/14/17 ? 😂

[ArtemSarmini]

Simple workaround exists

{
    auto stored_foo = foo();
    for (auto& x : stored_foo.items()) { /* .. */ }
}

it doesn't answer if items should be ref-qualified or not.

[nlohmann]

Any ideas how to proceed here? Maybe adding a note to the documentation of items?

[ArtemSarmini]

I think so. I mean proxy nature of items() is a feature, not a bug.

[BOT-Man-JL]

adding a note to the documentation

I hope so. It would be very helpful, especially for the users of dynamic languages (like Python).