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

JsonDocument dynamic allocation problem #1480

Closed
Leo-C opened this issue Jan 26, 2021 · 7 comments
Closed

JsonDocument dynamic allocation problem #1480

Leo-C opened this issue Jan 26, 2021 · 7 comments
Labels
bug v6 ArduinoJson 6

Comments

@Leo-C
Copy link

Leo-C commented Jan 26, 2021

Hi,
the title not refers to DynamicJsonDocument, but it refers precisely to dinamic allocation of a JsonDocument with new !

My environment is:
Platform: ESP32 TTGO
IDE: Arduino IDE 1.8.13

The simple code below compile well, but crash at delete:

#include <Arduino.h>
#include <ArduinoJson.h>

void setup()
{
	Serial.begin(115200);
}

void loop()
{
	Serial.println("Mem before: " + String(ESP.getFreeHeap()));
	
	JsonDocument* doc = new DynamicJsonDocument(1024);
	delete doc;
	
	Serial.println("Mem after: " + String(ESP.getFreeHeap()));
}

(this is a minimal example for a scenario where I allocate dinamically a JsonDocument into a function, return a pointer, and only after use, I call destructor in a different scope with delete in a different scope)

I appreciate any help to understand my error

Many thanks in advance

Leonardo

@Leo-C
Copy link
Author

Leo-C commented Jan 26, 2021

Sorry,
I've found that following code works:

#include <Arduino.h>
#include <ArduinoJson.h>

void setup()
{
	Serial.begin(115200);
}

void loop()
{
	Serial.println("Mem before: " + String(ESP.getFreeHeap()));
	
	DynamicJsonDocument* doc = new DynamicJsonDocument(1024);
	delete doc;
	
	Serial.println("Mem after: " + String(ESP.getFreeHeap()));
}

(only changing doc from type JsonDocument to DynamicJsonDocument)

Then, JsonDocument is not a superclass of DynamicJsonDocument? or destructor is not virtual?

Many thanks

Leonardo

@bblanchon
Copy link
Owner

Hi Leonardo,

Thank you very much for opening this issue.

DynamicJsonDocument is not intended to be deleted from a pointer to base.
In fact, it has no virtual function at all, and I don't want to pay the overhead just for this edge case.
As you'll see, I made the destructor of JsonDocument not virtual but protected, so the compiler will issue an error next time you try to delete a JsonDocument.

Now, I don't understand why anyone would want to do that since DynamicJsonDocument is already in the heap.
It seems that you want to return a JsonDocument from a function, right?
Why don't you write something like this:

DynamicJsonDocument makeJsonDocument() {
  DynamicJsonDocument doc(1024);
  // ...
  return doc;
}

DynamicJsonDocument is moveable, so no copy of the buffer is done, even if RVO doesn't kick in.

Best regards,
Benoit

@bblanchon bblanchon added the bug label Jan 27, 2021
@Leo-C
Copy link
Author

Leo-C commented Jan 27, 2021

Many Thanks!

I admit that I did not know Copy-Elision, then your suggestion is good and efficient.

I also have attempted to write a function like:

JsonDocument makeJsonDocument() {
  DynamicJsonDocument doc(1024);
  // ...
  return doc;
}

but was not accepted by compiler ...

Leonardo

@bblanchon
Copy link
Owner

Yes, that's expected.
Allowing this would result in a slicing of DynamicJsonDocument and therefore to a memory leak.

@smuellener
Copy link

smuellener commented Jan 30, 2021

One question, does that mean that DynamicJsonDocument has built-in smart shared pointer functionality? In other words:

std::shared_ptr<DynamicJsonDocument> create_a_doc_for_usage()
{
    auto doc = std::make_shared<DynamicJsonDocument>(JSON_OBJECT_SIZE(1));
    (*doc)["Content"] = "Test";
    return doc;
}

would not bring any benefit over

DynamicJsonDocument create_a_doc_for_usage()
{
    auto doc = DynamicJsonDocument(JSON_OBJECT_SIZE(1));
    doc["Content"] = "Test";
    return doc;
}

Thank you for the clarification!

@bblanchon
Copy link
Owner

@smuellener, in a sense, it's more like unique_ptr because it doesn't implement reference counting.

@bblanchon
Copy link
Owner

ArduinoJson 6.17.3 includes the protected destructor.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2021
@bblanchon bblanchon added the v6 ArduinoJson 6 label Feb 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug v6 ArduinoJson 6
Projects
None yet
Development

No branches or pull requests

3 participants