search

pupnp / pupnp

libupnp: Build UPnP-compliant control points, devices, and bridges on several operating systems.
https://pupnp.github.io/pupnp
BSD 3-Clause "New" or "Revised" License
349 stars 114 forks source link

fix memory leak and ActionExAsync #426

Closed philippe44 closed 1 year ago

philippe44 commented 1 year ago

This intends to fix https://github.com/pupnp/pupnp/issues/424. I've found at least 2 memory leaks

1- When sending Action(Ex)Async The ActionRequest document was not released nor was the ActionResult. I initially tought that this should be done in UpnpActionComplete::UpnpActionComplete_delete() but I was wrong because ActionRequest and ActionResult do not "belong" to the Evt object. The sets methods only set a pointer and modifying generator.c is not the right solution. The reason is that ActionRequest "belongs" to the ACTION job and should be handled as such if we want as well that, when the job is cancelled, the memory is released as well. So I've change the free_func for ACTION to reflect that.

2- When using event subscription The Event was not released upon Timer termination. So I've done as above, created a special free_func that handles that properly.

In general, when something prevents the execution/submission of a job, I've also used the actual free function instead of duplicating the individual free(). Note that when terminating a timer, I'm using the job's internal structure free_func() as it was already used to access arg. One can argue that it should be an opaque structure and get/set or the option to call the free function could be use instead.

Also, I realized that ActionExAsync has never been working properly because when the job is executed, only an Action is made, never an ActionEx although the Header document is in the job's argument. I've fixed that as well and stopped there to not go too deep in that rabbit hole.

I've also removed the upnp_timeout.c/h files as they were not used anymore (and had a strange names anyway)

whyman commented 1 year ago

Nice work @philippe44!

mrjimenez commented 1 year ago

Nice work @philippe44!

Indeed! :+1:

philippe44 commented 1 year ago

Thanks - I've been using it for a week no w/o issues. I'm surprised that nobody had been bitten by that before!

mrjimenez commented 1 year ago

Thanks - I've been using it for a week no w/o issues. I'm surprised that nobody had been bitten by that before!

Indeed, surprising. Thanks for the work.

Sorry for taking so long to do a release.

Best regards, Marcelo.