[Tile-serving] [openstreetmap/mod_tile] Examine memory usage for potential leaks (Issue #445)

[Hummeltech]

> Hm, I can see this possibility for the fault: I tried to find a way to distinguish if a statistics thread was created or not by initializing the value of `stats_thread` with a "null value". Apparently this doesn't exist portably, instead one should use the value of `pthread_self()` and do a `pthreads_equal()` [comparison](https://stackoverflow.com/questions/6276939/is-there-an-invalid-pthread-t-id). However, according to [the man page](https://man7.org/linux/man-pages/man3/pthread_create.3.html#RETURN_VALUE) (and also as _qbert220_ points out), the value is undefined upon return, so a proper implementation should also re-initialize the variable upon creation failure (in addition to the initialization before checking if a statistics threads is requested at all). Why the creation would fail is a different question though...

Thanks for the explanation, that sounds like it might be a good addition. The `stats` thread is optional, so that is at least one reason it would not be created, any other errors should lead to an exit.

> In either case, the statistics thread is also sitting in a hard `sleep()` and that should be rewritten to use a timed condition sleep: sleep for the requested time OR when a exit has been requested. But since this was just testing code to figure out any post-mortem leaks, I chose the "Can't be bothered with this detail now"-way.

Yes, there are a number of potential improvements that can be made, 

> These days, `renderd` does not terminate cleanly (the signal handlers didn't work for me, they seem to be a mechanism for the render threads to indicate a catastrophy (e.g. style/font loading failed)). Wouldn't the proper way be to respond to a signal? I'd imagine that's what e.g. `systemd` would send to the process e.g. during a shutdown/reboot and nowadays it just gets a `kill` signal after some time of not responding to it...? How important would that be as a feature?

You are correct that this did not exist for a very long time (and still does not in the latest release `v0.7.2`), it was, however, added recently into the default branch [here](https://github.com/openstreetmap/mod_tile/commit/ff06794c006bbe4523dbad911d883131b8394c0c#diff-3f59a73c820d2e95c60403cfca31fe5de38a3de58bbd2968b4b04d62f107805f).

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/mod_tile/issues/445#issuecomment-2200365367
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/mod_tile/issues/445/2200365367 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/tile-serving/attachments/20240701/fe1407d4/attachment.htm>