[Asterisk-code-review] json: Audit ast json * usage for thread safety. (asterisk[13])

[Matt Jordan]

Matt Jordan has posted comments on this change.

Change subject: json: Audit ast_json_* usage for thread safety.
......................................................................


Patch Set 1:

> We've got a mix of both uses, with older code using o and an
 > explicit ast_json_ref. I'd rather we pick one and stick with it.

I completely agree with Josh.

The major concern I have with our prior usage isn't necessarily whether or not a particular usage was safe or not, it is whether or not a developer would reasonably understand if their usage of 'O' is safe. The locking that occurs in the ast_json_* wrappers is purposefully abstracted away from the caller. As a result, the developer is left with the understanding that they should not care about the thread-safety of the JSON library - it has been taken care of for them. As Josh pointed out, that isn't always the case.

In those cases where the abstraction cannot provide thread safety, we absolutely should discourage their use. There's little harm in setting a precedent throughout the code base on the "right way" to pack a JSON object in another.

-- 
To view, visit https://gerrit.asterisk.org/1828
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I06550d8b0cc1bfeb56cab580a4e608ae4f1ec7d1
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: No