diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..cd06dd8a --- /dev/null +++ b/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 --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) +``` diff --git a/includes/render_config.h b/includes/render_config.h index cbadae0f..084db6f4 100644 --- a/includes/render_config.h +++ b/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) diff --git a/src/gen_tile.cpp b/src/gen_tile.cpp index d91769d5..87a6082f 100644 --- a/src/gen_tile.cpp +++ b/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 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 ren(map_parameterized, buf, map->scale); - ren.apply(); - } else { - mapnik::agg_renderer ren(map->map, buf, map->scale); - ren.apply(); - } + mapnik::agg_renderer 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()); diff --git a/src/renderd.c b/src/renderd.c index 81e0bda1..06c6be50 100644 --- a/src/renderd.c +++ b/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,10 +680,27 @@ 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; @@ -681,7 +708,7 @@ void *slave_thread(void * arg) 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])) { diff --git a/src/request_queue.c b/src/request_queue.c index 91a1358a..6bee27c1 100644 --- a/src/request_queue.c +++ b/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; diff --git a/src/store_file.c b/src/store_file.c index b1409aa7..91c7bfaa 100644 --- a/src/store_file.c +++ b/src/store_file.c @@ -74,6 +74,11 @@ static int file_tile_read(struct storage_backend * store, const char *xmlconfig, struct meta_layout *m = (struct meta_layout *)malloc(header_len); size_t file_offset, tile_size; + if (m == NULL) { + snprintf(log_msg, PATH_MAX - 1, "Failed to allocate memory for metatile header\n"); + return -1; + } + meta_offset = xyzo_to_meta(path, sizeof(path), store->storage_ctx, xmlconfig, options, x, y, z); fd = open(path, O_RDONLY); @@ -218,7 +223,13 @@ static int file_metatile_write(struct storage_backend * store, const char *xmlco xyzo_to_meta(meta_path, sizeof(meta_path), (char *)(store->storage_ctx), xmlconfig, options, x, y, z); g_logger(G_LOG_LEVEL_DEBUG, "Creating and writing a metatile to %s", meta_path); - tmp = malloc(sizeof(char) * strlen(meta_path) + 24); + tmp = malloc(strlen(meta_path) + 24); + + if (tmp == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "file_metatile_write: failed to allocate memory for temp path"); + return -1; + } + snprintf(tmp, strlen(meta_path) + 24, "%s.%lu", meta_path, (unsigned long) pthread_self()); if (mkdirp(tmp)) { @@ -245,7 +256,14 @@ static int file_metatile_write(struct storage_backend * store, const char *xmlco } close(fd); - rename(tmp, meta_path); + + if (rename(tmp, meta_path) != 0) { + g_logger(G_LOG_LEVEL_WARNING, "Error renaming temp file %s to %s: %s", tmp, meta_path, strerror(errno)); + unlink(tmp); + free(tmp); + return -1; + } + free(tmp); return sz; diff --git a/src/store_memcached.c b/src/store_memcached.c index dd296717..e9710496 100644 --- a/src/store_memcached.c +++ b/src/store_memcached.c @@ -80,6 +80,11 @@ static int memcached_tile_read(struct storage_backend * store, const char *xmlco memcached_return_t rc; char * buf_raw; + if (m == NULL) { + snprintf(log_msg, 1024, "Failed to allocate memory for metatile header\n"); + return -1; + } + mask = METATILE - 1; meta_offset = (x & mask) * METATILE + (y & mask); diff --git a/src/store_rados.c b/src/store_rados.c index 4d3cd16a..aae7c838 100644 --- a/src/store_rados.c +++ b/src/store_rados.c @@ -134,6 +134,11 @@ static int rados_tile_read(struct storage_backend * store, const char *xmlconfig int err; char * buf_raw; + if (m == NULL) { + snprintf(log_msg, 1024, "Failed to allocate memory for metatile header\n"); + return -1; + } + mask = METATILE - 1; meta_offset = (x & mask) * METATILE + (y & mask); @@ -234,6 +239,11 @@ static int rados_metatile_write(struct storage_backend * store, const char *xmlc char * buf2 = malloc(sz2); int err; + if (buf2 == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "rados_metatile_write: failed to allocate memory for write buffer"); + return -1; + } + tile_stat.expired = 0; tile_stat.size = sz; tile_stat.mtime = time(NULL); @@ -351,7 +361,7 @@ struct storage_backend * init_storage_rados(const char * connection_string) int err; int i; - if (ctx == NULL) { + if (ctx == NULL || store == NULL) { free(ctx); free(store); return NULL; diff --git a/src/store_ro_composite.c b/src/store_ro_composite.c index 998be917..a11acc13 100644 --- a/src/store_ro_composite.c +++ b/src/store_ro_composite.c @@ -248,7 +248,23 @@ struct storage_backend * init_storage_ro_composite(const char * connection_strin } connection_string_secondary = strstr(connection_string, "}{"); + + if (connection_string_secondary == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_composite: malformed connection string, missing '}{' separator: %s", connection_string); + free(ctx); + free(store); + return NULL; + } + connection_string_primary = malloc(strlen(connection_string) - strlen("composite:{") - strlen(connection_string_secondary) + 1); + + if (connection_string_primary == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_composite: failed to allocate memory for primary connection string"); + free(ctx); + free(store); + return NULL; + } + memcpy(connection_string_primary, connection_string + strlen("composite:{"), strlen(connection_string) - strlen("composite:{") - strlen(connection_string_secondary)); connection_string_primary[strlen(connection_string) - strlen("composite:{") - strlen(connection_string_secondary)] = 0; connection_string_secondary = strdup(connection_string_secondary + 2); @@ -258,18 +274,41 @@ struct storage_backend * init_storage_ro_composite(const char * connection_strin g_logger(G_LOG_LEVEL_DEBUG, "init_storage_ro_composite: Secondary storage backend: %s", connection_string_secondary); tmp = strstr(connection_string_primary, ","); + + if (tmp == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_composite: malformed primary connection string, missing ',' separator: %s", connection_string_primary); + free(connection_string_primary); + free(connection_string_secondary); + free(ctx); + free(store); + return NULL; + } + memcpy(ctx->xmlconfig_primary, connection_string_primary, tmp - connection_string_primary); ctx->xmlconfig_primary[tmp - connection_string_primary] = 0; ctx->store_primary = init_storage_backend(tmp + 1); if (ctx->store_primary == NULL) { g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_composite: failed to initialise primary storage backend"); + free(connection_string_primary); + free(connection_string_secondary); free(ctx); free(store); return NULL; } tmp = strstr(connection_string_secondary, ","); + + if (tmp == NULL) { + g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_composite: malformed secondary connection string, missing ',' separator: %s", connection_string_secondary); + free(connection_string_primary); + free(connection_string_secondary); + ctx->store_primary->close_storage(ctx->store_primary); + free(ctx); + free(store); + return NULL; + } + memcpy(ctx->xmlconfig_secondary, connection_string_secondary, tmp - connection_string_secondary); ctx->xmlconfig_secondary[tmp - connection_string_secondary] = 0; ctx->store_secondary = init_storage_backend(tmp + 1); diff --git a/src/store_ro_http_proxy.c b/src/store_ro_http_proxy.c index dabcf6ca..aeb0183d 100644 --- a/src/store_ro_http_proxy.c +++ b/src/store_ro_http_proxy.c @@ -116,6 +116,7 @@ static int ro_http_proxy_tile_retrieve(struct storage_backend * store, const cha if (res != CURLE_OK) { g_logger(G_LOG_LEVEL_ERROR, "ro_http_proxy_tile_fetch: failed to retrieve file: %s", curl_easy_strerror(res)); + free(chunk.memory); ctx->cache.x = -1; ctx->cache.y = -1; ctx->cache.z = -1; @@ -126,6 +127,7 @@ static int ro_http_proxy_tile_retrieve(struct storage_backend * store, const cha if (res != CURLE_OK) { g_logger(G_LOG_LEVEL_ERROR, "ro_http_proxy_tile_fetch: failed to retrieve HTTP code: %s", curl_easy_strerror(res)); + free(chunk.memory); ctx->cache.x = -1; ctx->cache.y = -1; ctx->cache.z = -1; @@ -300,6 +302,7 @@ struct storage_backend * init_storage_ro_http_proxy(const char * connection_stri if (res != CURLE_OK) { g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_http_proxy: failed to initialise global curl: %s", curl_easy_strerror(res)); + free(ctx->baseurl); free(ctx); free(store); return NULL; @@ -309,6 +312,7 @@ struct storage_backend * init_storage_ro_http_proxy(const char * connection_stri if (!ctx->ctx) { g_logger(G_LOG_LEVEL_ERROR, "init_storage_ro_http_proxy: failed to initialise curl"); + free(ctx->baseurl); free(ctx); free(store); return NULL;