cmcfarlen commented on code in PR #13658:
URL: https://github.com/apache/trafficserver/pull/13658#discussion_r3973699325


##########
src/iocore/cache/unit_tests/test_CacheShm.cc:
##########
@@ -246,7 +246,14 @@ namespace
 {
 
 // A prefix of our own so these tests can never touch a real instance's 
segments.
-constexpr const char *PURGE_PREFIX_WORD = "atspurgetest";
+// Per process: POSIX shm names are system global and every TEST_CASE runs as 
its own
+// ctest process, so a fixed word would let concurrent cases fight over one 
segment.
+std::string const &
+purge_prefix_word()
+{
+  static std::string const word{"atspurgetest" + std::to_string(getpid())};
+  return word;
+}

Review Comment:
   Same as the `test_CacheShmShutdown.cc` thread — shortened to `atspurge` in 
0993e1446d with the length budget noted. See that thread for the arithmetic; 
`atspurgetest` was the tighter of the two at only two characters of headroom.
   



##########
src/iocore/cache/unit_tests/test_CacheShm.cc:
##########
@@ -327,7 +334,7 @@ segment_size(const std::string &name)
 // the name space, or every stripe segment leaks while `traffic_ctl cache shm 
clear` reports success.
 TEST_CASE("CacheShm purge sweeps by name when the control layout is foreign", 
"[cache][shm]")
 {
-  const std::string prefix      = 
cache_shm::normalize_name_prefix(PURGE_PREFIX_WORD);
+  const std::string prefix      = 
cache_shm::normalize_name_prefix(purge_prefix_word());

Review Comment:
   Same as the `test_CacheShmShutdown.cc` thread — shortened to `atspurge` in 
0993e1446d with the length budget noted. See that thread for the arithmetic; 
`atspurgetest` was the tighter of the two at only two characters of headroom.
   



##########
src/iocore/cache/unit_tests/test_CacheShmShutdown.cc:
##########
@@ -48,14 +48,22 @@ namespace
 {
 
 // Our own prefix so these can never touch a real instance's segments, short 
enough to stay under the 31-char POSIX limit.
-constexpr const char *TEST_PREFIX_WORD = "atsunittest";
+// Per process: POSIX shm names are system global and every TEST_CASE runs as 
its own
+// ctest process, so a fixed word would let concurrent cases fight over one 
segment.
+std::string const &
+test_prefix_word()
+{
+  static std::string const word{"atsunittest" + std::to_string(getpid())};
+  return word;
+}
 
 // False when shm is unavailable here, e.g. a sandbox forbidding shm_open, so 
the test skips rather than fails.
 bool
 enable_shm()
 {
   REQUIRE(RecSetRecordInt("proxy.config.cache.shm.enabled", 1, 
REC_SOURCE_EXPLICIT) == REC_ERR_OKAY);
-  REQUIRE(RecSetRecordString("proxy.config.cache.shm.name_prefix", 
TEST_PREFIX_WORD, REC_SOURCE_EXPLICIT) == REC_ERR_OKAY);
+  REQUIRE(RecSetRecordString("proxy.config.cache.shm.name_prefix", 
test_prefix_word().c_str(), REC_SOURCE_EXPLICIT) ==
+          REC_ERR_OKAY);

Review Comment:
   Same as the `test_CacheShmShutdown.cc` thread — shortened to `atspurge` in 
0993e1446d with the length budget noted. See that thread for the arithmetic; 
`atspurgetest` was the tighter of the two at only two characters of headroom.
   



-- 
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]

Reply via email to