{"action":"created","issue":{"url":"https://api.github.com/repos/meetecho/janus-gateway/issues/403","repository_url":"https://api.github.com/repos/meetecho/janus-gateway","labels_url":"https://api.github.com/repos/meetecho/janus-gateway/issues/403/labels{/name}","comments_url":"https://api.github.com/repos/meetecho/janus-gateway/issues/403/comments","events_url":"https://api.github.com/repos/meetecho/janus-gateway/issues/403/events","html_url":"https://github.com/meetecho/janus-gateway/pull/403","id":120193441,"number":403,"title":"Reference counters","user":{"login":"lminiero","id":3684796,"avatar_url":"https://avatars.githubusercontent.com/u/3684796?v=3","gravatar_id":"","url":"https://api.github.com/users/lminiero","html_url":"https://github.com/lminiero","followers_url":"https://api.github.com/users/lminiero/followers","following_url":"https://api.github.com/users/lminiero/following{/other_user}","gists_url":"https://api.github.com/users/lminiero/gists{/gist_id}","starred_url":"https://api.github.com/users/lminiero/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lminiero/subscriptions","organizations_url":"https://api.github.com/users/lminiero/orgs","repos_url":"https://api.github.com/users/lminiero/repos","events_url":"https://api.github.com/users/lminiero/events{/privacy}","received_events_url":"https://api.github.com/users/lminiero/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":80,"created_at":"2015-12-03T15:14:30Z","updated_at":"2016-10-01T17:02:11Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/meetecho/janus-gateway/pulls/403","html_url":"https://github.com/meetecho/janus-gateway/pull/403","diff_url":"https://github.com/meetecho/janus-gateway/pull/403.diff","patch_url":"https://github.com/meetecho/janus-gateway/pull/403.patch"},"body":"This patch introduces a new file that provides a reference counter implementation that should hopefully be used across the whole Janus code, so that we can get rid of all the timed watchdogs/garbage collectors we have in place now (and the problems they can cause any time a race condition occurs), in both core and plugins/transports.\r\n\r\nThis PR does nothing but add this new implementation to the codebase. It's still not integrated in any part of Janus, as I first wanted to make sure this was a step in the right direction, and whether or not the mechanism itself as it is implemented right now can be improved. I didn't want to make use of GLib GObjects for several reasons, so I decided to take a lighter approach with something more \"homemade\". This excellent [blog post](http://nullprogram.com/blog/2015/02/17/) provided some really nice info on how this could be done in an effective way, and that's what lead to the code I just added, which is quite obviously heavily based on that: atomic operations on counters, and a callback function that the application sets and is called when the counter gets to 0.\r\n\r\nAccording to how the implementation works, now, this is how you'd use it. Assuming this is the struct we want to handle through the RC mechanism:\r\n\r\n\tstruct mystruct {\r\n\t\tint number;\r\n\t\tchar *string;\r\n\t}\r\n\r\nthat's how you'd modify it:\r\n\r\n\tstruct mystruct {\r\n\t\tint number;\r\n\t\tchar *string;\r\n\t\tjanus_refcount rc;\r\n\t}\r\n\r\nWhen allocating a new instance of that struct, you have to initialize the reference counter:\r\n\r\n\tstruct mystruct *instance = [..];\r\n\tjanus_refcount_init(&instance->rc, mystruct_free_cb);\r\n\r\nThis automatically sets the counter to 1 (as you obviously own yourself), and instructs the reference counter about which function must be invoked when the counter gets to 0 and so you can free the instance. The ```mystruct_free_cb``` method must have a signature like this:\r\n\r\n\tvoid mystruct_free_cb(janus_refcount *counter);\r\n\r\nand its implementation might look like this:\r\n\r\n\tvoid mystruct_free_cb(janus_refcount *counter) {\r\n\t\tstruct mystruct *instance = janus_refcount_containerof(counter, struct mystruct, rc);\r\n\t\tif(instance->string)\r\n\t\t\tfree(instance->string);\r\n\t\tfree(instance);\r\n\t}\r\n\r\nThe important part here is ```janus_refcount_containerof```, which is a macro we use to get the pointer to the instance that needs to be freed out of its reference counter. We pass it, in order:\r\n\r\n1. a pointer to the reference counter itself;\r\n2. the type of the instance (```struct mystruct``` in this case);\r\n3. the name of the reference counter as member of the instance struct (```rc``` in our example).\r\n\r\nThis is needed because, as explained in the mentioned blog post, you can make use of ```offsetof``` to programmatically calculate the pointer to the container instance starting from one of its members (our reference counter, in this case). This saves us the need to have another pointer (and so more bytes) in ```janus_refcount``` itself to keep track of the object instance we're handling.\r\n\r\nTo increase the counter, you just use ```janus_refcount_increase```:\r\n\r\n\tjanus_refcount_increase(&instance->rc);\r\n\r\nUnsurprisingly, ```janus_refcount_decrease``` decreases the counter instead:\r\n\r\n\tjanus_refcount_decrease(&instance->rc);\r\n\r\nBoth work on the actual counter using atomic operations, and so are inherently thread-safe. As soon as a call to ```janus_refcount_decrease``` causes the counter to become 0, the callback we set when initializing is invoked, and we can free the instance.\r\n\r\nThis is basically how it's supposed to work: if you're familiar with reference counters, nothing too fancy or new, then. If you think this is already in a good shape, I'll start integrating this in the code we have now. In our case this will have the additional complexity of objects containing objects containing objects and so on, all of them reference counted, likely (e.g., just think of session → handle → streams → components → dtls stack → sctp stack), but if we do this right, by making sure that each father increases the counter of its child and that \"consumers\" of those instances will also increase/decrease, this should all work.\r\n\r\nFeedback and suggestions welcome!"},"comment":{"url":"https://api.github.com/repos/meetecho/janus-gateway/issues/comments/250923630","html_url":"https://github.com/meetecho/janus-gateway/pull/403#issuecomment-250923630","issue_url":"https://api.github.com/repos/meetecho/janus-gateway/issues/403","id":250923630,"user":{"login":"andreasg123","id":15636985,"avatar_url":"https://avatars.githubusercontent.com/u/15636985?v=3","gravatar_id":"","url":"https://api.github.com/users/andreasg123","html_url":"https://github.com/andreasg123","followers_url":"https://api.github.com/users/andreasg123/followers","following_url":"https://api.github.com/users/andreasg123/following{/other_user}","gists_url":"https://api.github.com/users/andreasg123/gists{/gist_id}","starred_url":"https://api.github.com/users/andreasg123/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/andreasg123/subscriptions","organizations_url":"https://api.github.com/users/andreasg123/orgs","repos_url":"https://api.github.com/users/andreasg123/repos","events_url":"https://api.github.com/users/andreasg123/events{/privacy}","received_events_url":"https://api.github.com/users/andreasg123/received_events","type":"User","site_admin":false},"created_at":"2016-10-01T17:02:11Z","updated_at":"2016-10-01T17:02:11Z","body":"There was a race condition between hangup media and sending a packet to subscribers.  #640 should address that situation."}}
{"actor":{"display_login":"andreasg123"}}