masaori335 commented on code in PR #13328: URL: https://github.com/apache/trafficserver/pull/13328#discussion_r3687434303
########## src/iocore/cache/CacheShm.cc: ########## @@ -0,0 +1,776 @@ +/** @file + + Shared-memory-backed cache directory for fast restart. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "CacheShm.h" +#include "CacheShmLayout.h" +#include "CacheShmPurge.h" + +#include "P_CacheDir.h" +#include "iocore/cache/Store.h" + +#include "records/RecCore.h" +#include "tscore/Diags.h" +#include "tscore/HashFNV.h" +#include "tscore/hugepages.h" +#include "tscore/ink_align.h" +#include "tscore/ink_memory.h" +#include "tscore/ink_string.h" +#include "tsutil/DbgCtl.h" + +#include <fcntl.h> +#include <sys/file.h> +#include <sys/mman.h> +#include <sys/stat.h> +#include <unistd.h> + +#include <csignal> +#include <cstdint> +#include <cstring> +#include <mutex> +#include <string> +#include <unordered_map> + +namespace +{ + +DbgCtl dbg_ctl{"cache_shm"}; + +using cache_shm::CACHE_SHM_MAGIC; +using cache_shm::CACHE_SHM_SCHEMA_VERSION; +using cache_shm::CacheShmControl; +using cache_shm::control_segment_name; +using cache_shm::CONTROL_SIZE; +using cache_shm::LockResult; +using cache_shm::MAX_SHM_NAME_LEN; +using cache_shm::MAX_STRIPES; +using cache_shm::read_shm_name; +using cache_shm::StripeEntry; +using cache_shm::try_lock_control; + +// Sanity bound: the control struct (header + stripe table) must stay small. +constexpr std::size_t MAX_CONTROL_SEGMENT_BYTES = 32 * 1024; +static_assert(sizeof(CacheShmControl) <= MAX_CONTROL_SEGMENT_BYTES, "control segment unexpectedly large"); + +// Configuration loaded at initialize() time. +struct Config { + bool enabled = false; + bool use_hugepages = false; + bool purge_stale_on_start = false; + std::string name_prefix = "/ats-"; // normalized "/<word>-" (see normalize_name_prefix); set in load_config. +}; + +Config g_config; + +// Live state for the open control segment. +CacheShmControl *g_control = nullptr; +std::string g_control_name; + +// Holds the control segment's exclusive flock for the process lifetime; the OS +// releases it on exit. Only set on the path that owns the segment. +ats_scoped_fd g_control_fd; + +// shm pointers we returned (mapped to their length), so the Stripe destructor can +// choose munmap vs ats_free and detach_stripe can munmap the right span. +std::mutex g_pointers_mutex; +std::unordered_map<char *, std::size_t> g_pointers; + +// Guards the control-segment stripe table and the per-run claim bookkeeping below +// (stripes initialize concurrently across disk threads). +std::mutex g_table_mutex; + +// Per-run partial-attach bookkeeping, indexed in lockstep with g_control->stripes[]. +// An entry still unclaimed once init completes is an orphan reclaimed by +// finalize_attach(). Process-local, reset each run. +bool g_entry_claimed[MAX_STRIPES] = {}; +uint32_t g_claims_this_run = 0; + +void +fnv_update(ATSHash64FNV1a &h, uint64_t v) +{ + h.update(&v, sizeof v); +} + +/// Full 64-bit stripe identity used to match a stripe to its prior shm segment. +uint64_t +compute_stripe_key_hash(const char *stripe_key) +{ + ATSHash64FNV1a hash; + hash.update(stripe_key, std::strlen(stripe_key)); + return hash.get(); +} + +/// Build a stripe shm name from its per-host index (unique, so names never +/// collide). Matching to a prior segment uses the key hash, not this name. +std::string +build_stripe_shm_name(const std::string &prefix, uint32_t stripe_index) +{ + std::string name = prefix + "s" + std::to_string(stripe_index); + if (name.size() >= MAX_SHM_NAME_LEN) { + name.resize(MAX_SHM_NAME_LEN - 1); + } + return name; +} + +// Named flags for open_and_map_shm so call sites read `ShmAccess::Create` / +// `HugePages::Off` and the two can't be transposed. +enum class ShmAccess { Open, Create }; +enum class HugePages { Off, On }; + +/// Open or create a shm segment of `size` bytes and mmap it; nullptr on failure. +/// `out_fd` (if set) receives the still-open fd so the caller can flock it, else the +/// fd is closed (the mapping survives). `out_errno` (if set) receives the failing +/// syscall's errno for diagnostics. +void * +open_and_map_shm(const std::string &name, std::size_t size, ShmAccess access, [[maybe_unused]] HugePages hugepages, + int *out_fd = nullptr, int *out_errno = nullptr) +{ + if (out_errno != nullptr) { + *out_errno = 0; + } + int oflags = O_RDWR; + if (access == ShmAccess::Create) { + // O_EXCL so a create never adopts a pre-existing (attacker-planted) object. + oflags |= O_CREAT | O_EXCL; + } + + ats_scoped_fd fd{shm_open(name.c_str(), oflags, 0600)}; + if (fd < 0) { + int e = errno; + Dbg(dbg_ctl, "shm_open(%s, %s) failed: %s", name.c_str(), access == ShmAccess::Create ? "create" : "open", strerror(e)); + if (out_errno != nullptr) { + *out_errno = e; + } + return nullptr; + } + + if (access == ShmAccess::Create) { + if (ftruncate(fd, size) < 0) { + int e = errno; + Warning("ftruncate(%s, %zu) failed: %s", name.c_str(), size, strerror(e)); + shm_unlink(name.c_str()); + if (out_errno != nullptr) { + *out_errno = e; + } + return nullptr; + } + } else { + // The kernel rounds shm size up to a page boundary (16 KiB on macOS / Apple + // Silicon), so accept any size in [requested, page-up]. + struct stat sb { + }; + std::size_t expected_max = INK_ALIGN(size, ats_pagesize()); + if (fstat(fd, &sb) < 0 || sb.st_size < 0 || static_cast<std::size_t>(sb.st_size) < size || + static_cast<std::size_t>(sb.st_size) > expected_max) { Review Comment: Confirmed — the size-checked open returns nullptr, so initialize() skips the whole drop block and the O_EXCL create then fails with EEXIST on every restart. Took the second option. CONTROL_HEADER_SIZE = offsetof(CacheShmControl, stripes) is now a documented frozen prefix pinned by a static_assert, and a new open_control_segment() maps just that much when the segment's size is not this build's. The liveness guard (flock, or owner_pid where flock is unsupported) still applies, and then the segment is dropped and recreated in the same start: cache shm: control segment /ats-control is 20001 bytes, not this build's 12336; dropping it cache shm: creating fresh control segment /ats-control (owner pid ...) One thing your scenario also exposed: with the table unreadable the old drop path skipped the unlink loop entirely and leaked every stripe segment — a whole directory each, so potentially GBs of shm. Both the server drop path and purge_segments() now sweep <prefix>s0..MAX_STRIPES-1 by name when the table cannot be trusted (those names are ours by construction, so nothing outside the prefix is touched). New autest cache_shm_control_size_mismatch covers it: ts2 must drop + recreate, and then ts3 must fast-attach the segment ts2 created — that last step is what proves the wedge is gone rather than deferred by one restart. Addressed by 80cff77c80. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
