Prevent memory leak when exception is thrown in adl_serializer::to_json

[barcode]

(problem described in issue #3881)

There is a memory leak when throwing inside an adl serializers to_json method.

• [x] Add unit test

• [x] Implement fix

• [x] Fix Ci

• • *

Pull request checklist

Read the Contribution Guidelines for detailed information.

• [x] Changes are described in the pull request, or an existing issue is referenced.
• [x] The test suite compiles and runs without error.
• [x] Code coverage is 100%. Test cases can be added by editing the test suite.
• [x] The source code is amalgamated; that is, after making changes to the sources in the include/nlohmann directory, run make amalgamate to create the single-header files single_include/nlohmann/json.hpp and single_include/nlohmann/json_fwd.hpp. The whole process is described here.

[github-actions[bot]]

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated.

[coveralls]

Coverage: 100.0%. Remained the same when pulling 79bae8a96ab6061dfdcb388a3b7fe90c3f874b9d on barcode:fix_mem_leak_in_adl_serializer into b2306145e1789368e6f261680e8dc007e91cc986 on nlohmann:develop.

[barcode]

The test detects the memleak: https://github.com/nlohmann/json/actions/runs/3792554026/jobs/6448842970#step:5:316

[barcode]

I went with the solution using a member var, since it requires less changes to the existing code and the resulting code is simpler to understand.

[gregmarr]

Since this keeps most of the handling in the json class rather than the new data class, such as the copy constructor and move constructor, should the assert invariant function stay there as well? Also, should the copy and move constructors of this class be defaulted? Seems like they should be deleted.

[barcode]

should the assert invariant function stay there as well?

in that case the assert invariant method is not called if an exception in the ctor occurred. Then we do not need to adapt that method to deal with broken objects. Since the dtor of an element is called after the objects dtor finishes, the call order would be OK as well (assert before destroy).

Also, should the copy and move constructors of this class be defaulted? Seems like they should be deleted.

we need at least the move ctor https://github.com/nlohmann/json/blob/df39768dd0f0a488f363fabfb8a3c495263f751c/include/nlohmann/json.hpp#L1196

The others we could delete.

[barcode]

I looked a bit at the ci_test_compilers_clang (10) issue. This PR did not change the affected test and it worked in earlier builds.

It seems the compiler and changed to a newer version.

old:

Compiler: clang version 10.0.0; Target: x86_64-unknown-linux-gnu; Thread model: posix; InstalledDir: /usr/local/bin

new:

Compiler: Debian clang version 10.0.1-++20210313014605+ef32c611aa21-1~exp1~20210313125208.190; Target: x86_64-pc-linux-gnu; Thread model: posix; InstalledDir: /usr/bin

Since this issue has nothing to do with this PR. it probably should be fixed in a separate PR.

[barcode]

Work on this PR is done and it only needs a review.

[nlohmann]

Thanks!