From 3dac2a636a53d0429373dcedb6a1ef5bb5ec6bbd Mon Sep 17 00:00:00 2001 From: totaj Date: Wed, 15 Feb 2023 19:34:53 +0800 Subject: [PATCH] Add lock for loading so. --- src/common/backend/catalog/pg_proc.cpp | 6 +++ .../utils/cache/knl_localsysdbcache.cpp | 1 + src/common/backend/utils/fmgr/dfmgr.cpp | 42 ++++++++++++++++++- .../backend/utils/resowner/resowner.cpp | 30 ++++++++++++- src/common/port/gs_syscall_lock.cpp | 2 + src/gausskernel/process/main/main.cpp | 1 + src/gausskernel/process/stream/streamCore.cpp | 4 +- src/gausskernel/storage/ipc/ipc.cpp | 23 +++------- src/include/executor/executor.h | 16 ++++--- src/include/storage/ipc.h | 2 + src/include/utils/resowner.h | 3 +- src/include/utils/syscall_lock.h | 1 + 12 files changed, 102 insertions(+), 29 deletions(-) diff --git a/src/common/backend/catalog/pg_proc.cpp b/src/common/backend/catalog/pg_proc.cpp index 0783ff331..5a2a325db 100644 --- a/src/common/backend/catalog/pg_proc.cpp +++ b/src/common/backend/catalog/pg_proc.cpp @@ -2301,6 +2301,10 @@ void delete_file_handle(const char* library_path) DynamicFileList* file_scanner = NULL; DynamicFileList* pre_file_scanner = file_list; + AutoMutexLock libraryLock(&file_list_lock); + libraryLock.lock(); + + char* fullname = expand_dynamic_library_name(library_path); for (file_scanner = file_list; file_scanner != NULL; file_scanner = file_scanner->next) { if (strncmp(fullname, file_scanner->filename, strlen(fullname) + 1) == 0) { @@ -2323,6 +2327,8 @@ void delete_file_handle(const char* library_path) pre_file_scanner = file_scanner; } } + + libraryLock.unLock(); } /* diff --git a/src/common/backend/utils/cache/knl_localsysdbcache.cpp b/src/common/backend/utils/cache/knl_localsysdbcache.cpp index 056b8b009..160105aa1 100644 --- a/src/common/backend/utils/cache/knl_localsysdbcache.cpp +++ b/src/common/backend/utils/cache/knl_localsysdbcache.cpp @@ -1064,6 +1064,7 @@ LocalSysDBCache::LocalSysDBCache() void AtEOXact_SysDBCache(bool is_commit) { + ResourceOwnerReleasePthreadMutex(t_thrd.lsc_cxt.local_sysdb_resowner, is_commit); if (!EnableLocalSysCache()) { ResourceOwnerReleaseRelationRef(t_thrd.lsc_cxt.local_sysdb_resowner, is_commit); return; diff --git a/src/common/backend/utils/fmgr/dfmgr.cpp b/src/common/backend/utils/fmgr/dfmgr.cpp index dac5218ab..acad6a35a 100644 --- a/src/common/backend/utils/fmgr/dfmgr.cpp +++ b/src/common/backend/utils/fmgr/dfmgr.cpp @@ -28,6 +28,7 @@ #include "utils/hsearch.h" #include "utils/memutils.h" #include "utils/syscall_lock.h" +#include "postmaster/postmaster.h" #include "storage/file/fio_device.h" /* Max size of error message of dlopen */ @@ -167,6 +168,36 @@ PGFunction lookup_external_function(void* filehandle, const char* funcname) return (PGFunction)pg_dlsym(filehandle, funcname); } +/* + * release all library at proc exit. + */ +void internal_delete_library() +{ + DynamicFileList* file_scanner = NULL; + + AutoMutexLock libraryLock(&file_list_lock); + libraryLock.lock(); + + while (file_list != NULL) { + file_scanner = file_list; + file_list = file_list->next; +#ifndef ENABLE_MEMORY_CHECK + /* + * in the senario of ImmediateShutdown, it is not safe to close plugin + * as PM thread will not wait for all children threads exist(will send SIGQUIT signal) referring to pmdie + */ + if (g_instance.status != ImmediateShutdown && file_scanner->handle != NULL) { + (void)pg_dlclose(file_scanner->handle); + file_scanner->handle = NULL; + } +#endif + pfree((char*)file_scanner); + file_scanner = NULL; + } + file_list = file_tail = NULL; + libraryLock.unLock(); +} + /* * Load the specified dynamic-link library file, unless it already is * loaded. Return the pg_dl* handle for the file. @@ -186,6 +217,9 @@ void* internal_load_library(const char* libname) char* file = last_dir_separator(libname); file = (file == NULL) ? ((char*)libname) : (file + 1); + AutoMutexLock libraryLock(&file_list_lock); + libraryLock.lock(); + /* * Scan the list of loaded FILES to see if the file has been loaded. */ @@ -367,6 +401,8 @@ void* internal_load_library(const char* libname) u_sess->fmgr_cxt.file_init_tail = file_init_scanner; } + libraryLock.unLock(); + return file_scanner->handle; } @@ -490,8 +526,9 @@ static void internal_unload_library(const char* libname) * inode, else internal_load_library() will still think it's present. */ - AutoMutexLock libraryLock(&dlerror_lock); - + AutoMutexLock dlerrorLock(&dlerror_lock); + dlerrorLock.lock(); + AutoMutexLock libraryLock(&file_list_lock); libraryLock.lock(); for (file_scanner = file_list; file_scanner != NULL; file_scanner = nxt) { @@ -523,6 +560,7 @@ static void internal_unload_library(const char* libname) } libraryLock.unLock(); + dlerrorLock.unLock(); #endif /* NOT_USED */ } diff --git a/src/common/backend/utils/resowner/resowner.cpp b/src/common/backend/utils/resowner/resowner.cpp index e5320e857..41eee7d72 100755 --- a/src/common/backend/utils/resowner/resowner.cpp +++ b/src/common/backend/utils/resowner/resowner.cpp @@ -1799,7 +1799,7 @@ void PrintResourceOwnerLeakWarning() ereport(WARNING, (errmsg("resource owner \"%s\" may leak", IsolatedResourceOwner->name))); } -void ResourceOwnerReleasePthreadMutex() +void ResourceOwnerReleaseAllXactPthreadMutex() { ResourceOwner owner = t_thrd.utils_cxt.TopTransactionResourceOwner; ResourceOwner child; @@ -2012,6 +2012,23 @@ void ResourceOwnerForgetPthreadRWlock(ResourceOwner owner, pthread_rwlock_t* pRW errmsg("pthread rwlock is not owned by resource owner %s", owner->name))); } +int ResourceOwnerForgetIfExistPthreadMutex(ResourceOwner owner, pthread_mutex_t* pMutex, bool trace) +{ + pthread_mutex_t** mutexs = owner->pThdMutexs; + int ns1 = owner->nPthreadMutex - 1; + + if (!owner->valid) { + return 0; + } + + for (int i = ns1; i >= 0; i--) { + if (mutexs[i] == pMutex) { + return PthreadMutexUnlock(owner, pMutex, trace); + } + } + return 0; +} + void ResourceOwnerEnlargeLocalCatCList(ResourceOwner owner) { int newmax; @@ -2224,6 +2241,17 @@ void ResourceOwnerForgetGlobalBaseEntry(ResourceOwner owner, GlobalBaseEntry* en errmsg("the global base entry is not owned by resource owner %s", owner->name))); } +void ResourceOwnerReleasePthreadMutex(ResourceOwner owner, bool isCommit) +{ + while (owner->nPthreadMutex > 0) { + if (isCommit) { + PrintGlobalSysCacheLeakWarning(owner, "MutexLock"); + } + /* unlock do -- */ + PthreadMutexUnlock(owner, owner->pThdMutexs[owner->nPthreadMutex - 1]); + } +} + void ResourceOwnerReleaseRWLock(ResourceOwner owner, bool isCommit) { while (owner->nPthreadRWlock > 0) { diff --git a/src/common/port/gs_syscall_lock.cpp b/src/common/port/gs_syscall_lock.cpp index 5dca8957b..97cbc6970 100644 --- a/src/common/port/gs_syscall_lock.cpp +++ b/src/common/port/gs_syscall_lock.cpp @@ -32,6 +32,8 @@ syscalllock env_lock; syscalllock dlerror_lock; syscalllock kerberos_conn_lock; syscalllock read_cipher_lock; +syscalllock file_list_lock; + /* * @Description: Atomic set val into *ptr in a 32-bit address, and return the previous pointed by ptr diff --git a/src/gausskernel/process/main/main.cpp b/src/gausskernel/process/main/main.cpp index 68dc69c1a..838490343 100755 --- a/src/gausskernel/process/main/main.cpp +++ b/src/gausskernel/process/main/main.cpp @@ -544,4 +544,5 @@ static void syscall_lock_init(void) syscalllockInit(&dlerror_lock); syscalllockInit(&kerberos_conn_lock); syscalllockInit(&read_cipher_lock); + syscalllockInit(&file_list_lock); } diff --git a/src/gausskernel/process/stream/streamCore.cpp b/src/gausskernel/process/stream/streamCore.cpp index 2dafa16e0..9d3fd2278 100755 --- a/src/gausskernel/process/stream/streamCore.cpp +++ b/src/gausskernel/process/stream/streamCore.cpp @@ -904,7 +904,7 @@ void StreamNodeGroup::destroy(StreamObjStatus status) /* We must relase all pthread mutex by my thread, Or it will dead lock. But it is not a good solution. */ // lock the same thread mutex can't be conflict in one thread. - ResourceOwnerReleasePthreadMutex(); + ResourceOwnerReleaseAllXactPthreadMutex(); WaitState oldStatus = pgstat_report_waitstatus(STATE_STREAM_WAIT_NODEGROUP_DESTROY); @@ -954,7 +954,7 @@ void StreamNodeGroup::syncQuit(StreamObjStatus status) /* We must relase all pthread mutex by my thread, Or it will dead lock. But it is not a good solution. */ // lock the same thread mutex can't be conflict in one thread. - ResourceOwnerReleasePthreadMutex(); + ResourceOwnerReleaseAllXactPthreadMutex(); WaitState oldStatus = pgstat_report_waitstatus(STATE_STREAM_WAIT_THREAD_SYNC_QUIT); diff --git a/src/gausskernel/storage/ipc/ipc.cpp b/src/gausskernel/storage/ipc/ipc.cpp index 9f21760b5..20a153a31 100644 --- a/src/gausskernel/storage/ipc/ipc.cpp +++ b/src/gausskernel/storage/ipc/ipc.cpp @@ -157,8 +157,6 @@ void proc_exit_prepare(int code); */ void proc_exit(int code) { - DynamicFileList* file_scanner = NULL; - if (ENABLE_DMS && t_thrd.proc_cxt.MyProcPid == PostmasterPid) { // add cnt to avoid DmsCallbackThreadShmemInit to use UsedShmemSegAddr (void)pg_atomic_add_fetch_u32(&g_instance.dms_cxt.inProcExitCnt, 1); @@ -167,12 +165,15 @@ void proc_exit(int code) pg_usleep(WAIT_DMS_INIT_TIMEOUT); } } + if (t_thrd.utils_cxt.backend_reserved) { ereport(DEBUG2, (errmodule(MOD_MEM), errmsg("[BackendReservedExit] current thread role is: %d, used memory is: %d MB\n", t_thrd.role, t_thrd.utils_cxt.trackedMemChunks))); } + AtEOXact_SysDBCache(false); + audit_processlogout_unified(); (void)pgstat_report_waitstatus(STATE_WAIT_UNDEFINED); @@ -324,22 +325,8 @@ void proc_exit(int code) if (t_thrd.postmaster_cxt.redirection_done) ereport(LOG, (errmsg("Gaussdb exit(%d)", code))); - while (file_list != NULL) { - file_scanner = file_list; - file_list = file_list->next; -#ifndef ENABLE_MEMORY_CHECK - /* - * in the senario of ImmediateShutdown, it is not safe to close plugin - * as PM thread will not wait for all children threads exist(will send SIGQUIT signal) referring to pmdie - */ - if (g_instance.status != ImmediateShutdown) { - (void)pg_dlclose(file_scanner->handle); - } -#endif - pfree((char*)file_scanner); - file_scanner = NULL; - } - file_list = file_tail = NULL; + /* release all library at proc exit. */ + internal_delete_library(); if (u_sess->attr.attr_resource.use_workload_manager) gscgroup_free(); diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 1d39bfc87..1a53f71a7 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -483,6 +483,7 @@ extern Datum GetTypeZeroValue(Form_pg_attribute att_tup); extern Tuple ReplaceTupleNullCol(TupleDesc tupleDesc, TupleTableSlot* slot); extern Datum ExecEvalArrayRef(ArrayRefExprState* astate, ExprContext* econtext, bool* isNull, ExprDoneCond* isDone); +extern int ResourceOwnerForgetIfExistPthreadMutex(ResourceOwner owner, pthread_mutex_t* pMutex, bool trace); // AutoMutexLock // Auto object for non-recursive pthread_mutex_t lock @@ -491,7 +492,11 @@ class AutoMutexLock { public: AutoMutexLock(pthread_mutex_t* mutex, bool trace = true) : m_mutex(mutex), m_fLocked(false), m_trace(trace), m_owner(t_thrd.utils_cxt.CurrentResourceOwner) - {} + { + if (mutex == &file_list_lock) { + m_owner = t_thrd.lsc_cxt.local_sysdb_resowner; + } + } ~AutoMutexLock() { @@ -538,10 +543,11 @@ public: if (m_fLocked) { int ret = 0; - if (t_thrd.utils_cxt.CurrentResourceOwner == NULL) - m_owner = NULL; - - ret = PthreadMutexUnlock(m_owner, m_mutex, m_trace); + if (m_mutex == &file_list_lock) { + ret = ResourceOwnerForgetIfExistPthreadMutex(m_owner, m_mutex, m_trace); + } else { + ret = PthreadMutexUnlock(m_owner, m_mutex, m_trace); + } m_fLocked = (ret == 0 ? false : true); if (m_fLocked) { /* this should never happen, system may be completely in a mess */ diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h index 0e1922107..fd6185279 100644 --- a/src/include/storage/ipc.h +++ b/src/include/storage/ipc.h @@ -85,4 +85,6 @@ extern pthread_mutex_t gLocaleMutex; extern void CreateSharedMemoryAndSemaphores(bool makePrivate, int port); extern void cancelShmemExit(pg_on_exit_callback function, Datum arg); + +extern void internal_delete_library(); #endif /* IPC_H */ diff --git a/src/include/utils/resowner.h b/src/include/utils/resowner.h index f138ac49a..4b309ea30 100755 --- a/src/include/utils/resowner.h +++ b/src/include/utils/resowner.h @@ -153,7 +153,7 @@ extern void ResourceOwnerEnlargePthreadMutex(ResourceOwner owner); extern void PrintPthreadMutexLeakWarning(pthread_mutex_t* pMutex); extern void PrintResourceOwnerLeakWarning(); -extern void ResourceOwnerReleasePthreadMutex(); +extern void ResourceOwnerReleaseAllXactPthreadMutex(); extern void ResourceOwnerEnlargePartitionMapRefs(ResourceOwner owner); extern void ResourceOwnerRememberPartitionMapRef(ResourceOwner owner, PartitionMap* partmap); @@ -197,6 +197,7 @@ extern void ResourceOwnerEnlargeGlobalIsExclusive(ResourceOwner owner); extern void ResourceOwnerRememberGlobalIsExclusive(ResourceOwner owner, volatile uint32 *isexclusive); extern void ResourceOwnerForgetGlobalIsExclusive(ResourceOwner owner, volatile uint32 *isexclusive); +extern void ResourceOwnerReleasePthreadMutex(ResourceOwner owner, bool isCommit); extern void ResourceOwnerReleaseRWLock(ResourceOwner owner, bool isCommit); extern void ResourceOwnerReleaseLocalCatCTup(ResourceOwner owner, bool isCommit); extern void ResourceOwnerReleaseLocalCatCList(ResourceOwner owner, bool isCommit); diff --git a/src/include/utils/syscall_lock.h b/src/include/utils/syscall_lock.h index acc34ad64..c74e0e1a3 100644 --- a/src/include/utils/syscall_lock.h +++ b/src/include/utils/syscall_lock.h @@ -56,5 +56,6 @@ extern syscalllock env_lock; extern syscalllock dlerror_lock; extern syscalllock kerberos_conn_lock; extern syscalllock read_cipher_lock; +extern syscalllock file_list_lock; #endif /* SYSCALL_LOCK_H_ */