Resources cleanup upon using received data

[ferran]

Hi there,

I have advanced on this. The reason why the callback isn't executed is
that actually the socket hasn't been allocated. Capturing the error, I
get error code -6, that is,  NLE_EXIST, "Object exists" and system's
EEXIST or EADDRINUSE.

When I didn't do nl_socket_free and consequently, didn't close the
socket, nl lib kept assigning me sockets with different local ports.
However, if I do free the socket, for some nl lib tries to assign me a
socket with a local port that is still in use. This is my problem.

For some reason the library thinks that a local port is available when
it really isn't. What could motivate this misbehavior?

Regards,
Ferran

On 15/06/15 21:16, ferran wrote:
> Hi,
>
> I forgot to mention that the callback, when called, is always called
> twice. However, I don't free any resource inside the callback, but after it.
>
> I can't figure out how freeing the socket after the callback can prevent
> the callback itself to be called.
>
> Regards,
> Ferran
>
> On 15/06/15 20:31, ferran wrote:
>> Hi Dan,
>> Thank you for your kind reply.
>>
>> I agree that the nice way to go would be to reuse the same socket, but I
>> can't access to the upper level, in the example, "caller.c". It's part
>> of a wider project, and I'm afraid that it would break a level of
>> abstraction.
>>
>> I have to add information: "get_wireless_stats" is called once per
>> wireless device each 10 seconds. Therefore, "caller.c", in practice, is
>> doing:
>>
>> int main ()
>>     struct wireless_stats ofs;
>>     while (true)
>>     {
>>         for_each_dev(device)
>>         {
>>             get_wireless_stats(device, &ofs);
>>         }
>>         sleep_seconds(10);
>>     }
>> return 0;
>>
>> Where "device" is a string that is translated with "if_nametoindex"
>> inside "get_wireless_stats". Each device is queried one after the other
>> without threading or alike.
>>
>> This said, I tried again with more verbose logging and freed the msg and
>> socket resources before finishing "get_wireless_stats". I get the stats
>> doing :
>> ...
>>
>>     nlsmg_free(msg);
>>     nl_close(sk);
>>     return 0;
>>     
>> nla_put_failure:
>>     nlmsg_free(msg);
>>     nl_close(sk);
>>     return err;
>> }
>>
>> But if I do "nl_socket_free(sk)" instead of "nl_close(sk)", then the
>> callback isn't called, and the stats don't get copied to the target
>> structure. Does it make any sense to you?
>>
>> Regards,
>> Ferran
>>
>> On 12/06/15 18:33, Dan Williams wrote:
>>> On Fri, 2015-06-12 at 17:24 +0200, ferran wrote:
>>>> Hello everyone,
>>>> I have some issues at managing resources used to query wireless
>>>> statistics, I hope you can help me.
>>>>
>>>> Currently I successfully query cfg80211 to get statistics about wireless
>>>> devices such as tx bitrate, every 10 seconds. My problem is that I have
>>>> been unable to do this without leaking file descriptors and memory. The
>>>> code I use is inherited and I am trying to fix it, but I can't manage to
>>>> release the resources without losing the retrieved data. Until now, I
>>>> have read the documentation, compared with implementation examples found
>>>> here and there, and consulted frequently the doxygen reference. However,
>>>> I still need some more help.
>>>>
>>>> Issue 1: 'nl_recvmsgs_default()' will block until receiving a message if
>>>> socket is configured as blocking (default), but won't wait for the
>>>> callback to finish, right? So how could I possibly release the resources
>>>> just after nl_recvmsgs if the callback needs the received data?
>>> nl_recvmsgs_default() will block and process messages as long as (a)
>>> there are messages in the receive buffer and (b) your callback returns
>>> NL_SKIP.  It will wait for your callback to finish before proceeding.
>>>
>>> One thing I notice: there isn't a lot of sense in doing all the setup
>>> every 10 seconds.  You might as well just do the setup once and then
>>> every 10s do the nl_msg_alloc/send/recv.  
>>>
>>>> Issue 2: I have sumarized  the code so that you can check the run flow
>>>> and the memory management, it's posted in pastebin:
>>>> http://pastebin.com/EYADmDpy
>>>> Could you have a look to it, please? It's less than 100 lines, comments
>>>> included. How should I release the resources used by the nl socket and
>>>> nl message?
>>> The code you've got isn't freeing either 'msg' or 'sk', so you're
>>> leaking both of those every 10 seconds.  How about something like this;
>>> alternatively you can just make sure that both the normal and
>>> nla_put_failure paths in get_wireless_stats() free both 'msg' and 'sk'.
>>>
>>> //// ----------- caller.c ------------------
>>> /* includes */
>>> int main ()
>>>     struct wireless_stats ofs;
>>>     struct nl_sock *sk;
>>>     int family_id;
>>>     int err;
>>>
>>>     sk = nl_socket_alloc();
>>>     genl_connect(sk);
>>>
>>>     // WARNING, the docs say we must call genl_family_put() later and we don't!    
>>>     family_id = genl_ctrl_resolve(sk, "nl80211");
>>>     
>>>     nl_socket_modify_cb(sk, NL_CB_VALID, NL_CB_CUSTOM,
>>>         stats_handler, NULL);
>>>
>>>     while (true)
>>>     {
>>>         get_wireless_stats(sk, &ofs);
>>>         /* use retrieved data */
>>>     
>>>         sleep_seconds(10);
>>>     }
>>>
>>>     nl_socket_free(sk);
>>> return 0;
>>>
>>> //// --------- wireless_stats.c -----------
>>>
>>> void get_wireless_stats(struct nl_socket *sk, struct wireless_stats *ofs)
>>> {
>>>     struct nl_msg *msg;
>>>     int err;
>>>
>>>     shared_stats = ofs;
>>>     
>>>     msg = nlmsg_alloc();
>>>     genlmsg_put(msg, 0, 0, family_id, 0, NLM_F_DUMP, NL80211_CMD_GET_STATION, 0;
>>>     NLA_PUT_U32(msg, NL80211_ATTR_IFINDEX, if_nametoindex("mesh0"));
>>>     err = nl_send_auto_complete(sk, msg);
>>>     nl_recvmsgs_default(sk);
>>>     nlsmg_free(msg);
>>>     return 0;
>>>     
>>>     // Not sure who comes here, but it isn't part of the success workflow, and is required at compilation time.
>>>     // -> the NLA_PUT_U32 macro has a 'goto nla_put_failure' in case there isn't enough memory
>>> nla_put_failure:
>>>     nlmsg_free(msg);
>>>     return err;
>>> }
>>>
>>> <no changes to stats_handler>
>>>