cJSON is not multithreading safe

[tawmoto]

Hello,
I am trying to do some serialization in multiple threads but helgrind complains a lot, because of the above line.
Perhaps adding a compilation flag which disables this static global can be useful, because not everyone uses cJSON_GetErrorPtr function. The only solution as the moment is to patch the code internally before compiling it.

static error global_error = { NULL, 0 };

Thanks

[DGoldDragon28]

My suggestion to solve this would be to add the following new functions to the API whenever the next API change is:

CJSON_PUBLIC(cJSON *) cJSON_ParseWithOptsMT(const char *value, const char **return_parse_end, cJSON_bool require_null_terminated, error * err_ptr);
CJSON_PUBLIC(cJSON *) cJSON_ParseMT(const char *value, error * err_ptr);

With the following semantics: if err_ptr == NULL error locations will not be reported. Otherwise, the memory at err_ptr will be used to report the error.
Implementation is easy. Just take the non-MT implementation and replace the reference to global_error with a null-check and reference to *err_ptr

I would submit a PR, but I'm not sure if you are okay with API changes in the next release

[tawmoto]

Thank you for the fast reply, at the moment I deleted all references to the global error because I don't use it, but it would be nice to have it in the default implementation, so I don't have to patch it every time I update cjson to latest version.

I am ok with the API modifications in the next release, as I link cjson statically.

Thanks a lot!

[DGoldDragon28]

... The tone of your reply concerns me, as it seems you think I am in charge here. To be clear, I was speaking to the maintainer. I have no power here, was simply giving suggestions on a fix for the issue.

[tawmoto]

Oh sorry, I thought you were one of the maintainers :-)

[DGoldDragon28]

No problem, just wanted to make sure you weren't disappointed later on xD

[FSMaxB]

@tawmoto Please take a look at the README, there is a section about this issue: https://github.com/DaveGamble/cJSON#thread-safety

Essentially if you cJSON_ParseWithOpts, you get the error pointer as return_parse_end. And although the global error pointer might be written to concurrently in that case, it is never read thereafter, so it is fine.

[tawmoto]

Thank you, I know it's safe because I don't use that function, as I mentioned before. But the helgrind still is very noisy, the only possibility is to patch the code or add an exception to helgrind.

[FSMaxB]

For the time being that will probably be the only way to silence it. The internals of cJSON are already prepared to drop all global state once a new API comes around, but since I haven't found a lot of time to work on anything other than bugfixes it is not certain when that will be.