[Asterisk-code-review] ast json pack(): Use safer json ref mechanism. (asterisk[13])

[Corey Farrell]

Corey Farrell has posted comments on this change. ( https://gerrit.asterisk.org/7637 )

Change subject: ast_json_pack(): Use safer json ref mechanism.
......................................................................


Patch Set 1: Code-Review+1

> > I prefer the reference stealing form but what makes it safer?
 > 
 > The ast_json_pack() uses json_vpack_ex().  When json_vpack_ex()
 > gets its own ref for the 'O' option it uses the internal ref
 > function which is not thread safe.  Though if there happens to be
 > an OOM error packing the json object, it will use the internal
 > unref function which is not thread safe either.  However, the OOM
 > case should be quite rare.  Who runs out of memory. :)

The situations where json_vpack_ex does an unref will be expanding in v2.11.

ast_json_pack("{s:o, s:o}", "test1", NULL, "test2", ast_json_ref(blob));

This currently leaks because the processing stops at the NULL argument.  My patch to fix this leak in jansson has been merged so starting with v2.11 passing NULL where an object is expected will not leak blob but it will use the jansson unref.

Your patch does make the success case thread safe so that is good.


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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: I49204db2e57ae96eee43909c18ed007c09ac817e
Gerrit-Change-Number: 7637
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Tue, 19 Dec 2017 00:29:00 +0000
Gerrit-HasComments: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171218/df448ab1/attachment-0001.html>