[yugabyte#24578] DocDB: Fix TSAN failure in webserver

Summary:
Fixed concurrent access to access_logging_enabled and tcmalloc_logging_enabled.

Also fixed access to rpcz and ybrpczMemoryContext in webserver_worker_main in yb_pg_metrics.c.
Jira: DB-13615

Test Plan: ./yb_build.sh tsan --gtest_filter PgCatalogPerfTest.ResponseCacheInvalidationOnConnectionWithTempTableClosure

Reviewers: telgersma

Reviewed By: telgersma

Subscribers: ybase, yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D39336

Author: spolitov

src/postgres/yb-extensions/yb_pg_metrics/yb_pg_metrics.c

@@ -619,85 +619,85 @@ ws_sigterm_handler(SIGNAL_ARGS)
 void
 webserver_worker_main(Datum unused)
 {
-  YBCInitThreading();
-  /*
-   * We call YBCInit here so that HandleYBStatus can correctly report potential error.
-   */
-  HandleYBStatus(YBCInit(NULL /* argv[0] */, palloc, NULL /* cstring_to_text_with_len_fn */));
+	YBCInitThreading();
+	/*
+	* We call YBCInit here so that HandleYBStatus can correctly report potential error.
+	*/
+	HandleYBStatus(YBCInit(NULL /* argv[0] */, palloc, NULL /* cstring_to_text_with_len_fn */));
 
-  backendStatusArray = getBackendStatusArray();
+	backendStatusArray = getBackendStatusArray();
 
-  BackgroundWorkerUnblockSignals();
+	BackgroundWorkerUnblockSignals();
 
-  /*
-   * Assert that shared memory is allocated to backendStatusArray before this webserver
-   * is started.
-  */
-  if (!backendStatusArray)
-    ereport(FATAL,
-            (errcode(ERRCODE_INTERNAL_ERROR),
-             errmsg("Shared memory not allocated to BackendStatusArray before starting YSQL webserver")));
+	/*
+	* Assert that shared memory is allocated to backendStatusArray before this webserver
+	* is started.
+	*/
+	if (!backendStatusArray)
+		ereport(FATAL,
+			(errcode(ERRCODE_INTERNAL_ERROR),
+			 errmsg("Shared memory not allocated to BackendStatusArray before starting YSQL webserver")));
 
-  webserver = CreateWebserver(ListenAddresses, port);
+	webserver = CreateWebserver(ListenAddresses, port);
 
-  RegisterMetrics(ybpgm_table, num_entries, metric_node_name);
+	RegisterMetrics(ybpgm_table, num_entries, metric_node_name);
 
-  postgresCallbacks callbacks;
-  callbacks.pullRpczEntries      = pullRpczEntries;
-  callbacks.freeRpczEntries      = freeRpczEntries;
-  callbacks.getTimestampTz       = GetCurrentTimestamp;
-  callbacks.getTimestampTzDiffMs = getElapsedMs;
-  callbacks.getTimestampTzToStr  = timestamptz_to_str;
+	postgresCallbacks callbacks;
+	callbacks.pullRpczEntries      = pullRpczEntries;
+	callbacks.freeRpczEntries      = freeRpczEntries;
+	callbacks.getTimestampTz       = GetCurrentTimestamp;
+	callbacks.getTimestampTzDiffMs = getElapsedMs;
+	callbacks.getTimestampTzToStr  = timestamptz_to_str;
 
-  YbConnectionMetrics conn_metrics;
-  conn_metrics.max_conn = &MaxConnections;
-  conn_metrics.too_many_conn = yb_too_many_conn;
-  conn_metrics.new_conn = yb_new_conn;
+	YbConnectionMetrics conn_metrics;
+	conn_metrics.max_conn = &MaxConnections;
+	conn_metrics.too_many_conn = yb_too_many_conn;
+	conn_metrics.new_conn = yb_new_conn;
 
-  RegisterRpczEntries(&callbacks, &num_backends, &rpcz, &conn_metrics);
-  HandleYBStatus(StartWebserver(webserver));
+	RegisterRpczEntries(&callbacks, &num_backends, &rpcz, &conn_metrics);
+	HandleYBStatus(StartWebserver(webserver));
 
 	pqsignal(SIGHUP, ws_sighup_handler);
 	pqsignal(SIGTERM, ws_sigterm_handler);
 
-  SetWebserverConfig(webserver, log_accesses, log_tcmalloc_stats,
-                     webserver_profiler_sample_freq_bytes);
-
-  int rc;
-  while (!got_SIGTERM)
-  {
-    rc = WaitLatch(MyLatch, WL_LATCH_SET | WL_POSTMASTER_DEATH, -1, PG_WAIT_EXTENSION);
-    ResetLatch(MyLatch);
+	SetWebserverConfig(webserver, log_accesses, log_tcmalloc_stats,
+					   webserver_profiler_sample_freq_bytes);
 
-		if (rc & WL_POSTMASTER_DEATH)
-			break;
-
-    if (got_SIGHUP)
-    {
-      got_SIGHUP = false;
-      ProcessConfigFile(PGC_SIGHUP);
-      SetWebserverConfig(webserver, log_accesses, log_tcmalloc_stats,
-                         webserver_profiler_sample_freq_bytes);
-    }
-  }
-
-  if (rpcz != NULL && ybrpczMemoryContext != NULL)
-  {
-    MemoryContext oldcontext = MemoryContextSwitchTo(ybrpczMemoryContext);
-    pfree(rpcz);
-    MemoryContextSwitchTo(oldcontext);
-  }
+	int rc;
+	while (!got_SIGTERM)
+	{
+		rc = WaitLatch(MyLatch, WL_LATCH_SET | WL_POSTMASTER_DEATH, -1, PG_WAIT_EXTENSION);
+		ResetLatch(MyLatch);
+
+			if (rc & WL_POSTMASTER_DEATH)
+				break;
+
+		if (got_SIGHUP)
+		{
+			got_SIGHUP = false;
+			ProcessConfigFile(PGC_SIGHUP);
+			SetWebserverConfig(webserver, log_accesses, log_tcmalloc_stats,
+							   webserver_profiler_sample_freq_bytes);
+		}
+	}
 
 	if (webserver)
 	{
 		DestroyWebserver(webserver);
 		webserver = NULL;
 	}
 
-  if (rc & WL_POSTMASTER_DEATH)
-      proc_exit(1);
+	if (rpcz != NULL && ybrpczMemoryContext != NULL)
+	{
+		MemoryContext oldcontext = MemoryContextSwitchTo(ybrpczMemoryContext);
+		pfree(rpcz);
+		MemoryContextSwitchTo(oldcontext);
+	}
+
+	if (rc & WL_POSTMASTER_DEATH)
+		proc_exit(1);
 
-  proc_exit(0);
+	proc_exit(0);
 }
 
 /*

src/yb/server/webserver.cc

@@ -136,9 +136,6 @@ class Webserver::Impl {
 
   Status GetInputHostPort(HostPort* hp) const;
 
-  bool access_logging_enabled = false;
-  bool tcmalloc_logging_enabled = false;
-
   void RegisterPathHandler(const std::string& path, const std::string& alias,
                                    const PathHandlerCallback& callback,
                                    bool is_styled = true,
@@ -157,6 +154,11 @@ class Webserver::Impl {
 
   bool ContainsFlag(const std::string& flag) const EXCLUDES(flags_mutex_);
 
+  void SetLogging(bool enable_access_logging, bool enable_tcmalloc_logging) {
+    access_logging_enabled_.store(enable_access_logging, std::memory_order_release);
+    tcmalloc_logging_enabled_.store(enable_tcmalloc_logging, std::memory_order_release);
+  }
+
  private:
   // Container class for a list of path handler callbacks for a single URL.
   class PathHandler {
@@ -267,6 +269,9 @@ class Webserver::Impl {
   // this server needs. This is used to filter out the flags that are shown in the varz UI.
   std::unordered_set<std::string> auto_flags_ GUARDED_BY(flags_mutex_);
   std::unordered_set<std::string> process_flags_ GUARDED_BY(flags_mutex_);
+
+  std::atomic<bool> access_logging_enabled_ = false;
+  std::atomic<bool> tcmalloc_logging_enabled_ = false;
 };
 
 Webserver::Impl::Impl(const WebserverOptions& opts, const std::string& server_name)
@@ -589,7 +594,7 @@ sq_callback_result_t Webserver::Impl::BeginRequestCallback(struct sq_connection*
     handler = it->second;
   }
 
-  if (access_logging_enabled) {
+  if (access_logging_enabled_.load(std::memory_order_acquire)) {
     string params = request_info->query_string ? Format("?$0", request_info->query_string) : "";
     LOG(INFO) << "webserver request: " << request_info->uri << params;
   }
@@ -598,7 +603,7 @@ sq_callback_result_t Webserver::Impl::BeginRequestCallback(struct sq_connection*
   MemTracker::GcTcmallocIfNeeded();
 
 #if YB_TCMALLOC_ENABLED
-  if (tcmalloc_logging_enabled)
+  if (tcmalloc_logging_enabled_.load(std::memory_order_acquire))
     LOG(INFO) << "webserver tcmalloc stats:"
               << " heap size bytes: " << GetTCMallocPhysicalBytesUsed()
               << ", total physical bytes: " << GetTCMallocCurrentHeapSizeBytes()
@@ -867,8 +872,7 @@ Status Webserver::GetInputHostPort(HostPort* hp) const {
 }
 
 void Webserver::SetLogging(bool enable_access_logging, bool enable_tcmalloc_logging) {
-  impl_->access_logging_enabled = enable_access_logging;
-  impl_->tcmalloc_logging_enabled = enable_tcmalloc_logging;
+  impl_->SetLogging(enable_access_logging, enable_tcmalloc_logging);
 }
 
 void Webserver::RegisterPathHandler(const std::string& path,