Changes to fix memory leaks in renderd and mod_tile when using Postgres

CLAUDE.md

@@ -0,0 +1,156 @@
+# CLAUDE.md
+
+This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
+
+## Project Overview
+
+**mod_tile** is a high-performance tile serving system with two components:
+- **mod_tile**: Apache 2 HTTP module that serves map tiles
+- **renderd**: Daemon that renders tiles using Mapnik
+
+## Build System
+
+CMake is the primary build system (Autotools is the alternative). The project requires C99 and C++11 (C++17 for Mapnik 4+).
+
+### Ubuntu dependencies
+
+```sh
+sudo apt --no-install-recommends --yes install \
+  apache2 apache2-dev cmake curl g++ gcc git \
+  libcairo2-dev libcurl4-openssl-dev libglib2.0-dev \
+  libiniparser-dev libmapnik-dev libmemcached-dev librados-dev
+```
+
+### CMake Build (recommended)
+
+```sh
+export CMAKE_BUILD_PARALLEL_LEVEL=$(nproc)
+
+cmake -B /tmp/mod_tile_build -S . \
+  -DCMAKE_BUILD_TYPE=Release \
+  -DCMAKE_INSTALL_LOCALSTATEDIR=/var \
+  -DCMAKE_INSTALL_PREFIX=/usr \
+  -DCMAKE_INSTALL_RUNSTATEDIR=/run \
+  -DCMAKE_INSTALL_SYSCONFDIR=/etc \
+  -DENABLE_TESTS=ON
+
+cmake --build /tmp/mod_tile_build
+
+# Run all tests
+cd /tmp/mod_tile_build && ctest
+
+# Run a single test by name
+cd /tmp/mod_tile_build && ctest -R <test_name> --output-on-failure
+
+# Install
+sudo cmake --install /tmp/mod_tile_build --strip
+```
+
+### Autotools Build
+
+```sh
+./autogen.sh
+./configure
+make
+sudo make install
+```
+
+### Key CMake Options
+
+| Option | Default | Description |
+|--------|---------|-------------|
+| `ENABLE_TESTS` | OFF | Build test suite (Catch2) |
+| `USE_CAIRO` | ON | Cairo composite backend |
+| `USE_CURL` | ON | HTTP proxy storage backend |
+| `USE_MEMCACHED` | ON | Memcached storage backend |
+| `USE_RADOS` | ON | Ceph RADOS storage backend |
+| `MALLOC_LIB` | libc | Memory allocator: libc/jemalloc/mimalloc/tcmalloc |
+
+### Linting / Static Analysis
+
+CI runs `flawfinder` for security scanning (`.github/workflows/flawfinder-analysis.yml`) and a lint workflow (`.github/workflows/lint.yml`).
+
+## Architecture
+
+### Communication Flow
+
+```
+HTTP Client → mod_tile (Apache module)
+                  ├── Storage backends (tile cache hit) → Response
+                  └── Unix socket → renderd daemon
+                                        └── Request queue (5 priority levels)
+                                              └── Thread pool → Mapnik → Metatile storage
+```
+
+### Key Components
+
+**`src/mod_tile.c`** — Apache module: request handling, cache expiry heuristics, delay pool rate limiting, statistics. Config structures: `tile_config_rec` (per-directory), `tile_server_conf` (server-wide).
+
+**`src/renderd.c`** — Rendering daemon main loop; manages worker threads and Unix socket listener.
+
+**`src/gen_tile.cpp`** — Mapnik tile generation. Called by renderd worker threads.
+
+**`src/request_queue.c`** — Priority queue with hash-indexed deduplication. 5 levels: Normal, Priority, Low, Bulk, Dirty.
+
+**`src/renderd_config.c`** — Parses `renderd.conf` (INI format) into `renderd_config` / `xmlconfigitem` structs.
+
+**Storage backends** (pluggable via `includes/store.h` function-pointer interface):
+- `store_file.c` — filesystem (default), stores 8×8 metatiles
+- `store_memcached.c` — Memcached
+- `store_rados.c` — Ceph RADOS
+- `store_ro_http_proxy.c` — HTTP proxy (read-only)
+- `store_ro_composite.c` — composite read-only (requires Cairo)
+- `store_null.c` — no-op
+
+### Protocol
+
+mod_tile and renderd communicate over a Unix socket (default: `/run/renderd/renderd.sock`, TCP fallback: `localhost:7654`) using the protocol defined in `includes/protocol.h`. Protocol version is v3. Commands: `cmdRender`, `cmdDirty`, `cmdRenderPrio`, `cmdRenderLow`, `cmdRenderBulk`, `cmdDone`, `cmdNotDone`.
+
+### Metatile Format
+
+Tiles are stored in 8×8 metatile bundles (`METATILE = 8`) in a hashed directory structure. File format: `"META"` magic header + index of per-tile offsets/sizes. `includes/metatile.h` defines the layout; `src/metatile.cpp` is the C++ wrapper.
+
+### Important Constants (`includes/render_config.h`, `includes/mod_tile.h`)
+
+- `MAX_ZOOM = 20`
+- `HASHIDX_SIZE = 2213` (request deduplication hash)
+- Default tile directory: `/var/cache/renderd/tiles`
+- `MAX_LOAD_OLD = 16`, `MAX_LOAD_MISSING = 50` — re-render thresholds
+
+## Tests
+
+Tests use **Catch2** (v2.13.9) and live in `tests/`. The main suites:
+
+- `gen_tile_test.cpp` — Mapnik rendering pipeline (largest suite)
+- `renderd_config_test.cpp` — configuration parsing
+- `renderd_test.cpp` — daemon core
+- `render_expired_test.cpp`, `render_list_test.cpp`, `render_old_test.cpp` — utility programs
+- `render_speedtest_test.cpp` — performance tool
+
+Test infrastructure uses `tests/httpd.conf.in` and `tests/renderd.conf.in` templates to spin up live Apache + renderd for integration tests. `tests/tiles.sha256sum` holds expected checksums for tile output validation.
+
+## Known Issues / Technical Debt
+
+- **`src/request_queue.c:request_queue_close`** — queued render items are not freed on shutdown (items in all five priority lists leak). The TODO comment is present in the source. Safe in practice because renderd only shuts down at process exit, but should be fixed for clean valgrind runs.
+- **`src/store_ro_http_proxy.c:strcpy` at line 165** — `xmlconfig` is copied into `ctx->cache.xmlname[XMLCONFIG_MAX]` (41 bytes) without a prior length check. The caller is the storage backend interface which in practice receives validated xmlconfig names, but the copy is not bounds-safe.
+- **`src/renderd.c` / `src/mod_tile.c` — `bzero` usage** — several files use the deprecated `bzero()` instead of `memset(..., 0, ...)`. Functionally equivalent on Linux but not strictly portable.
+- **`src/gen_tile.cpp:render_thread` — startup `strndup`/`malloc` leaks** — `output_format`, `xmlfile`, `xmlname` (`strndup`), `prj` (`malloc`), and `store` (`init_storage_backend`) are allocated once per thread at startup and never freed. The render thread runs in an infinite loop and never exits, so these do not accumulate in practice.
+- **`src/store_ro_composite.c` — `connection_string_secondary` (strdup) not freed on late error paths** — after the `strdup` on the secondary connection string, several subsequent error paths (store_primary init failure, store_secondary init failure) free it, but if `store_secondary` init succeeds and a later step fails the pointer may still leak depending on the code path. Low risk: composite storage is rarely used and init failures abort the process.
+
+## Parameterized rendering cache
+
+When `parameterize_style` is configured (e.g. for multilingual maps), `render()` in `src/gen_tile.cpp` maintains a per-`xmlmapconfig` cache (`parameterized_map_cache`) mapping options strings to pre-built `mapnik::Map` copies. This prevents Mapnik's PostGIS datasource plugin from creating a new PostgreSQL connection pool entry on every render, which previously caused rapid memory exhaustion. The cache is per render-thread and unbounded in size; in practice the number of distinct options values is small (e.g. one per requested language).
+
+## Repository Layout
+
+```
+includes/       — public headers (protocol, store interface, config structs)
+src/            — C/C++ source for mod_tile, renderd, storage backends, utilities
+tests/          — Catch2 test suites + fixtures
+cmake/          — custom Find*.cmake modules
+docs/build/     — per-distro build instructions
+docs/man/       — man pages
+etc/            — example Apache and renderd config files
+utils/          — example map data
+.github/        — CI workflows (build-and-test, lint, flawfinder, docker)
+```

includes/render_config.h

@@ -121,7 +121,6 @@
 #define QUEUE_MAX (1024)
 #define REQ_LIMIT (512)
 #define DIRTY_LIMIT (10000)
-#define HASHIDX_SIZE 22123
 #endif
 
 // Penalty for client making an invalid request (in seconds)

src/gen_tile.cpp

@@ -89,6 +89,11 @@ struct projectionconfig {
 
 struct xmlmapconfig {
 	Map map;
+	/* Cache of parameterized Map copies keyed by the options string.
+	 * Avoids recreating PostGIS datasource objects (and their connection
+	 * pools) on every render, which would accumulate in Mapnik's global
+	 * ConnectionManager and exhaust memory under sustained load. */
+	std::map<std::string, mapnik::Map> parameterized_map_cache;
 	const char *host;
 	const char *htcphost;
 	const char *output_format;
@@ -247,31 +252,43 @@ static enum protoCmd render(struct xmlmapconfig *map, int x, int y, int z, char
 	unsigned int render_size_tx = MIN(METATILE, map->prj->aspect_x * (1 << z));
 	unsigned int render_size_ty = MIN(METATILE, map->prj->aspect_y * (1 << z));
 
-	map->map.resize(render_size_tx * map->tilesize, render_size_ty * map->tilesize);
-	map->map.zoom_to_box(tile2prjbounds(map->prj, x, y, z));
+	/* Select the map to render with: a cached parameterized copy, or the base map. */
+	mapnik::Map *render_map;
+
+	if (map->parameterize_function) {
+		/* Look up or create a parameterized map for this options string.
+		 * Re-using a cached copy avoids calling set_datasource() on every
+		 * render, which would create a new PostGIS connection pool entry in
+		 * Mapnik's global ConnectionManager each time and leak memory. */
+		auto it = map->parameterized_map_cache.find(std::string(options));
+
+		if (it == map->parameterized_map_cache.end()) {
+			mapnik::Map parameterized = map->map;
+			map->parameterize_function(parameterized, options);
+			parameterized.load_fonts();
+			auto inserted = map->parameterized_map_cache.emplace(std::string(options), std::move(parameterized));
+			it = inserted.first;
+			g_logger(G_LOG_LEVEL_DEBUG, "Cached new parameterized map for options '%s' (cache size: %zu)",
+				 options, map->parameterized_map_cache.size());
+		}
 
-	if (map->map.buffer_size() == 0) { // Only set buffer size if the buffer size isn't explicitly set in the mapnik stylesheet.
-		map->map.set_buffer_size((map->tilesize >> 1) * map->scale);
+		render_map = &it->second;
+	} else {
+		render_map = &map->map;
 	}
 
-	// m.zoom(size+1);
+	render_map->resize(render_size_tx * map->tilesize, render_size_ty * map->tilesize);
+	render_map->zoom_to_box(tile2prjbounds(map->prj, x, y, z));
+
+	if (render_map->buffer_size() == 0) { // Only set buffer size if the buffer size isn't explicitly set in the mapnik stylesheet.
+		render_map->set_buffer_size((map->tilesize >> 1) * map->scale);
+	}
 
 	mapnik::image_32 buf(render_size_tx * map->tilesize, render_size_ty * map->tilesize);
 
 	try {
-		if (map->parameterize_function) {
-			Map map_parameterized = map->map;
-
-			map->parameterize_function(map_parameterized, options);
-
-			map_parameterized.load_fonts();
-
-			mapnik::agg_renderer<mapnik::image_32> ren(map_parameterized, buf, map->scale);
-			ren.apply();
-		} else {
-			mapnik::agg_renderer<mapnik::image_32> ren(map->map, buf, map->scale);
-			ren.apply();
-		}
+		mapnik::agg_renderer<mapnik::image_32> ren(*render_map, buf, map->scale);
+		ren.apply();
 	} catch (std::exception const &ex) {
 		g_logger(G_LOG_LEVEL_ERROR, "failed to render TILE %s %d %d-%d %d-%d", map->xmlname, z, x, x + render_size_tx - 1, y, y + render_size_ty - 1);
 		g_logger(G_LOG_LEVEL_ERROR, "  reason: %s", ex.what());

src/renderd.c

@@ -170,14 +170,11 @@ enum protoCmd rx_request(struct protocol *req, int fd)
 
 void request_exit(void)
 {
-	// Any write to the exit pipe will trigger a graceful exit
+	// Any write to the exit pipe will trigger a graceful exit.
+	// This function is called from a signal handler, so only async-signal-safe
+	// functions (write(2)) may be used here — no g_logger, no strerror.
 	char c = 0;
-
-	g_logger(G_LOG_LEVEL_INFO, "Sending exit request");
-
-	if (write(exit_pipe_fd, &c, sizeof(c)) < 0) {
-		g_logger(G_LOG_LEVEL_ERROR, "Failed to write to the exit pipe: %s", strerror(errno));
-	}
+	(void)write(exit_pipe_fd, &c, sizeof(c));
 }
 
 void process_loop(int listen_fd)
@@ -589,7 +586,20 @@ void *slave_thread(void * arg)
 	struct protocol * req_slave;
 
 	req_slave = (struct protocol *)malloc(sizeof(struct protocol));
+
+	if (req_slave == NULL) {
+		g_logger(G_LOG_LEVEL_ERROR, "slave_thread: failed to allocate memory for req_slave");
+		return NULL;
+	}
+
 	resp = (struct protocol *)malloc(sizeof(struct protocol));
+
+	if (resp == NULL) {
+		g_logger(G_LOG_LEVEL_ERROR, "slave_thread: failed to allocate memory for resp");
+		free(req_slave);
+		return NULL;
+	}
+
 	bzero(req_slave, sizeof(struct protocol));
 	bzero(resp, sizeof(struct protocol));
 
@@ -670,18 +680,35 @@ void *slave_thread(void * arg)
 			retry = 10;
 
 			while ((ret_size < sizeof(struct protocol)) && (retry > 0)) {
-				ret_size = recv(pfd, resp + ret_size, (sizeof(struct protocol)
-								       - ret_size), 0);
+				ssize_t n = recv(pfd, (char *)resp + ret_size,
+						 sizeof(struct protocol) - ret_size, 0);
+
+				if (n < 0) {
+					if (errno == EINTR) {
+						continue;
+					}
 
-				if ((errno == EPIPE) || ret_size == 0) {
+					if (errno == EPIPE) {
+						close(pfd);
+						pfd = FD_INVALID;
+						ret_size = 0;
+						g_logger(G_LOG_LEVEL_ERROR, "Pipe to Renderd slave closed");
+						break;
+					}
+
+					retry--;
+					continue;
+				}
+
+				if (n == 0) {
 					close(pfd);
 					pfd = FD_INVALID;
 					ret_size = 0;
 					g_logger(G_LOG_LEVEL_ERROR, "Pipe to Renderd slave closed");
 					break;
 				}
 
-				retry--;
+				ret_size += n;
 			}
 
 			if (ret_size < sizeof(struct protocol)) {
@@ -875,6 +902,12 @@ int main(int argc, char **argv)
 
 	render_threads = (pthread_t *) malloc(sizeof(pthread_t) * config.num_threads);
 
+	if (render_threads == NULL) {
+		g_logger(G_LOG_LEVEL_CRITICAL, "Failed to allocate memory for render threads");
+		close(fd);
+		return 7;
+	}
+
 	for (i = 0; i < config.num_threads; i++) {
 		if (pthread_create(&render_threads[i], NULL, render_thread, (void *)maps)) {
 			g_logger(G_LOG_LEVEL_CRITICAL, "Could not spawn rendering thread");
@@ -888,6 +921,12 @@ int main(int argc, char **argv)
 		k = 0;
 		slave_threads = (pthread_t *) malloc(sizeof(pthread_t) * num_slave_threads);
 
+		if (slave_threads == NULL) {
+			g_logger(G_LOG_LEVEL_CRITICAL, "Failed to allocate memory for slave threads");
+			close(fd);
+			return 7;
+		}
+
 		for (i = 1; i < MAX_SLAVES; i++) {
 			for (j = 0; j < config_slaves[i].num_threads; j++) {
 				if (pthread_create(&slave_threads[k++], NULL, slave_thread, (void *) &config_slaves[i])) {

src/request_queue.c

@@ -491,6 +491,15 @@ struct request_queue * request_queue_init()
 	queue->renderHead.next = queue->renderHead.prev = &(queue->renderHead);
 	queue->hashidxSize = HASHIDX_SIZE;
 	queue->item_hashidx = (struct item_idx *) malloc(sizeof(struct item_idx) * queue->hashidxSize);
+
+	if (queue->item_hashidx == NULL) {
+		g_logger(G_LOG_LEVEL_ERROR, "Failed to allocate memory for request queue hash index");
+		pthread_cond_destroy(&(queue->qCond));
+		pthread_mutex_destroy(&(queue->qLock));
+		free(queue);
+		return NULL;
+	}
+
 	bzero(queue->item_hashidx, sizeof(struct item_idx) * queue->hashidxSize);
 
 	return queue;