crash in QJsonDocument::fromJson

[ChrisW67]

Welcome to the forum.

You appear to have a common C++ programming mistake: holding/returning a reference to temporary object.

jsonObj refers to the temporary QJsonObject returned by jsonDoc.object(). QJsonObject is an implicitly shared class, and shares the data with the document it has been created from. Both jsondoc and the temporary object are destroyed at function exit. Using the reference afterward is a bad idea.

[JayceGao]

@ChrisW67
Hi ChrisW67,

Thanks for your welcome and your input! I appreciate you taking the time to look at my code.

I understand your concern about returning a reference to a temporary object, and in general C++ programming, you are absolutely right, it's usually a bad practice and can lead to dangling references.

However, in this specific case with Qt's QJsonObject, I believe the situation is different due to its implicit sharing (copy-on-write) mechanism. jsonDoc.object() does return a temporary QJsonObject, but assigning it to jsonObj (which is a reference parameter) doesn't actually mean jsonObj is directly referencing a temporary object that will be immediately destroyed and become invalid.

Because QJsonObject is implicitly shared, the assignment jsonObj = jsonDoc.object(); results in jsonObj sharing the data of the JSON object with jsonDoc. It's not a deep copy in the traditional sense, but rather a shallow copy and a reference count increment. The underlying JSON data will be valid as long as either jsonDoc or jsonObj (or any other QJsonObject sharing the same data) is alive. Since jsonObj is passed by reference and exists in the calling scope outside of this function, the data it refers to will remain valid even after getObjFromJson returns and jsonDoc is destroyed.

Therefore, I believe in this specific scenario, the code is safe and doesn't suffer from the dangling reference issue you mentioned, precisely because of QJsonObject's implicit sharing.

Regarding the crash I'm experiencing, I suspect it might be caused by something else, perhaps issues within the JSON data itself causing problems during parsing within Qt's internal functions like QLocaleData::bytearrayToLongLong or QJsonPrivate::Parser::parseNumber. The crash stack seems to point towards number parsing issues within Qt's JSON parsing logic.

Could you (or anyone else) perhaps shed more light on whether my understanding of QJsonObject's implicit sharing in this context is correct? And if so, any ideas on what else might be causing the crash based on the stack trace?

Thanks again for your help!

Best regards,

JayceGao

[Christian Ehrlicher]

@JayceGao said in crash in QJsonDocument::fromJson:

Therefore, I believe in this specific scenario, the code is safe and doesn't suffer from the dangling reference issue you mentioned, precisely because of QJsonObject's implicit sharing.

You are wrong. References and implicit sharing are two different things. Fix your code.
Assigning something to a reference does nothing with the object itself and therefore no refcount can be increased or decreased.

[JayceGao]

Hi again,

Thanks for the continued discussion. To provide more context, here's how I typically use this function in my code:

QString json = ""; // some json text
QJsonObject json_obj;
bool parsed = getObjFromJson(json, json_obj);

// other code using json_obj ...
C++
This pattern is used extensively throughout my project, and in the vast majority of cases, it works without any issues. The crash I reported is an occasional occurrence, which makes me wonder if this common usage pattern itself could be problematic, even if it seems to work most of the time.

Could you please advise if this typical usage scenario might still have potential issues, despite the implicit sharing of QJsonObject? Or is it more likely that the occasional crashes are indeed related to the content of the json string being parsed, or perhaps some other factor unrelated to the function's structure and temporary object handling?

Thank you again for your insights.

Best regards,

JayceGao

[JonB]

@JayceGao
Your code as shown is fine, so long as you only use the QJsonObject json_obj; and its corresponding QJsonDocument jsonDoc within the scope it is declared, e.g. if this is in a function until the end of that function. What is wrong in your original is that you have a reference parameter QJsonObject& jsonObj to your function, and that gets assigned to the result of jsonDoc.object(), which itself is an object in the QJsonDocument jsonDoc. Nothing copies that object. So on exit from the function it references an object which has been destroyed.

It is possible that it "works" from time to time depending on what has happened to the memory holding the object since the function was exited. The behaviour is of course "undefined" ("UB"): as soon as the memory gets clobbered crashes are likely, if the memory happens not to have been overwritten/used elsewhere it will appear to work. That might be why you see an "intermittent" problem.

Btw, it is unlikely that Qt JSON code would "crash" on encountering malformed or problematic JSON text to parse. That would be a bug in Qt code. Rather you should find that the parse() (fromJson()) call returns failure and sets a QJsonParseError you can examine, if that happened.

[JayceGao]

@JonB Thanks I will check my code again.

[SGaist]

Hi,

One thing you could do is use the swap function.

Other possibilities, return the QJsonObject and pass the Boolean as output parameter (like for example the QString conversion functions).

Use std::optional for the return value so you don't need an output parameter.

[Christian Ehrlicher]

Since QJsonObject() has a function isEmpty() I don't see why you can't simply return a QJsonObject

[SGaist]

@Christian-Ehrlicher one possible reason that comes to mind: empty does not imply failure to load the json data and it might be a valid case.

[Axel Spoerl]

Return the QJsonObject and pass a pointer to a boolean to set a result, if necessary at all.

static QJsonObject getObjFromJson(const QString &json, bool *ok = nullptr)
{
    QJsonParseError jsonParseError;
    const QJsonDocument jsonDoc = QJsonDocument::fromJson(json.toUtf8(), &jsonParseError);
    const bool result = jsonParseError.error == QJsonParseError::NoError;
    if (ok)
        *ok = result;    
    return jsonDoc.object();
 }