Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please provide more precise informations about when to call json_object_put #642

Closed
fralbo opened this issue Jul 9, 2020 · 12 comments
Closed

Comments

@fralbo
Copy link

fralbo commented Jul 9, 2020

Hello,

It's difficult to determine when to call json_object_put after json_obect_xxx calls.
I found tons of questions about this and all examples and tutorials I found really don't care about correctly freeing the memory.
And answers and not allways clear as it seems that doubts are widely shared.

One example about json_object_parse_ex in your documentation, you effectively specify that an object is returned so we can logically think that a call to json_object_put is necessary but a clearer notice would be fine all the more the given code snippet doesn't mention json_object_put call. Maybe json_object_put documentation could give the list of the functions supposed to need this call.
I understand it's maybe not so simple as some functions transfer ownership and so, if I well understand because it's also not so obvious, doesn't wait from freeing the object from theirself.

What json_tokener_freedo ??

Some functions increment ownership such as json_object_get is it the only one?
Can json_object_get be called other function, like json_tokern_parse for example?
And so is there an json_object_count to call json_object_put until 0?

Often, functions which doesn't change increment count specify that there is no need to call json_object_put except for special situation.
But the ones which need it don't tell anything such as json_object_get even if it souds obvious for you.

And itseems that the documentation refers to 0.10 version.

Thanks in advance.

@dota17
Copy link
Member

dota17 commented Jul 9, 2020

It's an interesting topic. And I sorted out the related interfaces.

  • json_object_new_object()
    Create a new empty object with a reference count of 1. The caller of this object initially has sole ownership
  • json_object_get()
    Increment the reference count of json_object, thereby grabbing shared ownership of obj.
  • json_object_object_add()/json_object_object_add_ex(), json_object_object_get()/json_object_object_get_ex(),
    json_object_array_add(), json_object_array_put_idx()
    No reference counts will be changed.
  • json_object_object_del()
    The reference count will be decremented for the deleted object.
  • json_object_array_del_idx()
    The reference count will be decremented for each of the deleted objects.
  • json_object_put()
    Decrement the reference count of json_object and free if it reaches zero.

So, it looks like json_object_get() is the only one to increment the ownership. json_tokern_parse() calls the json_object_get() and increments the ownership of tok->stack->current. Because we need to call json_tokener_new() and new a json_tokener* tok, we also need to call json_tokener_free() to free the tok. At the same time, tok->stack->current will be free or the reference count -1.

Maybe we should provide more detailed messages about these in Readme.

@dota17
Copy link
Member

dota17 commented Jul 9, 2020

But, it is indeed a question. if we call json_object_object_add(obj1, "key2", obj2) and even json_object_put(obj1), we still could visit the obj2, as we have not set the obj2 as NULL.

@hawicz
Copy link
Member

hawicz commented Jul 9, 2020

No, you MUST NOT "visit" obj2 because as soon as you call json_object_put(obj1) obj2 is no longer valid. Trying to use it anyway is a use-after-free error, exactly as if you were directly to do char *p = malloc(10); free(p); *p = 'x';.

@ploxiln
Copy link
Contributor

ploxiln commented Jul 9, 2020

The tokener is independent from the json objects. You create the tokener, use it once, or multiple times, and free it when you are done with it. It is not tied to the lifetime of the objects it creates while parsing text, the objects live on.

Typically, you create a tree of json objects, either by parsing text with a tokener, or by creating-and-adding them one-by-one. Typically, every object in the tree will have one reference, from it's parent. When you are done with the tree of objects, you put() just the root object, and that put()s its child objects automatically, and that frees the entire tree in a cascading manner from a single put(). This is pretty simple. You can get a reference to a single child and copy its value in just a few lines, while obviously not freeing the parent, without dealing with any reference count changes, and this is safe.

You can take a reference to a sub-section of the object tree and use it for a non-trivial amount of time, perhaps after the original object tree was freed, by get()ing root of the sub-tree to increase it's "reference count" ... then you can treat it as an independent object tree. But you may not have to do this, depending on your application.

EDIT: I hereby license this comment text under the "CC0" license, to be used as freely as possible.

@hawicz
Copy link
Member

hawicz commented Jul 9, 2020

@fralbo, the Stackoverflow page you linked to explicitly links to the 0.10 docs, so of course you're going to see the old info. You should start at http://json-c.github.io/json-c/, and look at the 0.14 (soon to be 0.15) docs instead, there have been a TON of improvements, including specifying when reference counts are adjusted. Of course, there's always room for improvement.

json_tokener_free() frees a json_tokener object. That's it, nothing more. Yes, this could use some explicit docs.

json_object_get() is the only function that increments the ref count.

@hawicz
Copy link
Member

hawicz commented Jul 9, 2020

Thanks for that explanation @ploxiln. I feel like that should be part of a summary/intro section that we add as a "@mainpage" block somewhere, perhaps a "mainpage.dox", though I'm not quite sure how to include that in the doc build.

@dota17
Copy link
Member

dota17 commented Jul 10, 2020

No, you MUST NOT "visit" obj2

Yes, agree. It is a wrong action. But many users don not know this. It may lead them to free or visit the pointer and the valgrind throw an exception finally. They may be confused for this. So we need to emphasize these in doc.

@fralbo
Copy link
Author

fralbo commented Jul 12, 2020 via email

@hawicz
Copy link
Member

hawicz commented Jul 12, 2020

@fralbo , you can't call json_tokener_free() when you don't have a json_tokener! That is just basic C usage, please actually look at the types of the APIs you are referring to.

You free the returned json_object with json_object_put(), just like any other json_object.

Unless otherwise specified (i.e. only json_object_object_get_ex() or json_object_array_get_idx()) json-c functions that return a json_object allocate it (or increase its refcount) and thus you need to free it with json_object_put().
Incidentally, I just noticed the docs for json_object_array_get_idx() were somewhat lacking, and updated it: 2330c6f

@hawicz
Copy link
Member

hawicz commented Jul 12, 2020

@dota17 , I updated the doc for json_object_put() (and also json_object_get()) to clarify that objects shouldn't be accessed after being freed. (4d9f6dd)

@fralbo
Copy link
Author

fralbo commented Jul 13, 2020

@hawicz yes of course, sorry for the mistake.

@hawicz
Copy link
Member

hawicz commented Jul 20, 2020

With the updated docs on the functions, and the additional detail I added to README.md, I'm considering this done.
Also, there's at least one person that put together a tutorial, which you can see for additional detail: https://github.com/rbtylee/tutorial-jsonc

@hawicz hawicz closed this as completed Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants