bblanchon / ArduinoJson

📟 JSON library for Arduino and embedded C++. Simple and efficient.
https://arduinojson.org
MIT License
6.63k stars 1.1k forks source link

JsonDocument in queue #2066

Closed aetherium-ignis closed 3 months ago

aetherium-ignis commented 3 months ago

Description I want to pass the JsonDocument between tasks through a queue (FREERTOS), the problem is that now the size is dynamic just like it happens with the strings. The queue has a reserved size which should not exceed the JsonDocument (I am aware of the memory inefficiency of this), with the StaticJsonDocument it was possible to use the queues, for some reason although JsonDocument has a value character, the copy in the queue is not performs correctly, my intuition is that internally something like memcpy will be used for the copy to copy a block of memory from the indicated pointer and the copy of the JsonDocument does not support said copy, is there a way to pass the JsonDocument to the queue directly or Is it necessary to serialize to an array that serves as an intermediary?

Troubleshooter's report

  1. The program uses ArduinoJson 7
  2. The issue happens at compile time
  3. Error says "StaticJsonBuffer is a class from ArduinoJson 5"
  4. Program uses ArduinoJson directly
  5. User prefers upgrading to ArduinoJson 7
  6. Upgrading doesn't fix the issue

Environment

Reproduction code

QueueHandle_t queue = xQueueCreate(2, 4098);

Task1:
JsonDocument json;
json["test"] = 12;
xQueueSend(queue, &json, NULL);

Task2:
JsonDocument json;
xQueueReceive(queue, &json, pdMS_TO_TICKS(60000));
serializeJsonPretty(json, Serial);

Remarks I have left only the relevant lines. The output obtained is: {} The desired one is: {"test":12}

FredeEB commented 3 months ago

When you call xQueueSend you're doing a memcpy into the queue, so you're storing the address of the stack allocated object, ie. a pointer to where it is on the stack. Then when the JsonDocument object goes out of scope, its destructor is called, freeing the underlying stroage, which means that the object that the pointer you just sent to your queue is pointing to, is no longer there.

When task 2 comes around to getting it back off the stack, the object has already been deleted, and you're reading into uninitialized memory, which is undefined behavior by the C++ standard.

The way to fix this is by using the free store, so you allocate the JsonDocument object somewhere, where its destructor is not called before task 2 has the chance to read this. The most common way to achieve this, is by calling new (C++), and then passing the returned pointer through the queue, and then freeing it with delete when you're done with the JsonDocument, which also calls the destructor

// Task 1
void taskfunc1() {
    // ...
    JsonDocument* json = new JsonDocument();
    *json["test"] = 12;
    xQueueSend(queue, &json, NULL);
}

// Task 2
void taskfunc2() {
    // ...
    JsonDocument* json;
    xQueueReceive(queue, &json, pdMS_TO_TICKS(60000));
    serializeJsonPretty(*json, Serial);
    delete json;
}
aetherium-ignis commented 3 months ago

Thanks for the quick reply, I understand what you say about the destruction but the following is specified in the queues:

"Post an item to a queue. The item is queued by copy, not by reference."

This, as I understand it, uses the memory space reserved when you create the queue to create a copy of the original and precisely avoid referencing a destroyed object. The described effect would be correct if what is copied to the queue is a pointer, according to the documentation that would be a JsonObject or JsonVariant that points to the JsonDocument, the JsonDocument would be the container that contains the information, if it is copied to the queue it should carry the information and not just the reference to the original object. Without knowing internally how JsonDocument works, I cannot guarantee that this is the case. An array of, say, 10 positions, when passed to the queue, all 10 positions are copied. To be possible, a queue element must have at least 10 reserved positions. I put a example similar to the original to explain myself better:

QueueHandle_t queue = xQueueCreate(2, 10);

Task1:
char send_data[10] = {10, 11, 12};
xQueueSend(queue, send_data, NULL);

Task2:
char receive_data[10];
xQueueReceive(queue, receive_data, pdMS_TO_TICKS(60000));

In this example, when the send is done (& is not necessary in this case because it is an array) the content of the array is copied to the queue, even if send_data is destroyed, the copy remains in the queue so the data is not lost being available in the receive. Note that if the array is larger than the length reserved in the queue this will not be done correctly, but if the data passed were pointers the necessary size would only be 1 position.

I don't know how dynamic elements work, I don't know if they can be copied with memcpy or not because the data is not contiguous in memory or because we don't know the size, the queue should copy the size reserved after the pointer that is assigned to it. happens, whether it is an object or not.

FredeEB commented 3 months ago

I get why you think this is the case, but let me try and explain it better

The short answer is that objects that have a destructor, cannot be, what is called, trivially copied, ie copied via memcpy. So if you pass in an array with its size, it will decay to a pointer, meaning the address of the first element, and essentially call memcpy from that pointer and however many bytes forward that you specified.

If we take an std::string as an example, it could be implemented like this

class string {
    char* data;
    size_t size;
    size_t capacity;

    string(char const* value) {
        size = strlen(value);
        data = (char *)malloc(size);
        memcpy(data, value, size);
    }

    ~string() {
        free(data);
    }
};

Copying this structure will copy the data verbatim, meaning whatever value the data pointer has (eg 0x74616188) would be inserted into your queue. And as long as that object remains valid, that pointer will be valid, eg.

void foo() {
    {
        std::string data("horse");
        // data is valid here
        xQueueSend(queue, &data, size of(data));
    }
    // But NOT here, because the object went out of scope, and the destructor was called.
    // The compiler will call the destructor when data goes out of scope regardless of how many times you've memcpy'd it
}
aetherium-ignis commented 3 months ago

With this it is perfectly explained, I suspected that it would be something similar to the problem with the strings and this confirms it for me.

The solution provided in the first comment is perfectly valid although in my case I intend to share the same information with more than 1 task so I need to make several independent copies, I will choose to use an intermediate array to serialize the information, this array can be passed by copy taking advantage of the queues, it is similar to what I do in the case of the string. Another option would be to create several copies of the JsonDocument before sharing them and use the option that you indicated, this unfortunately implies not making mistakes when freeing the memory.

Thanks for the instructions, they were very helpful, for my part we are closing this thread.