public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v48 2/7] Add conditional lock feature to dshash 3+ messages / 3 participants [nested] [flat]
* [PATCH v48 2/7] Add conditional lock feature to dshash @ 2020-03-13 07:58 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Kyotaro Horiguchi @ 2020-03-13 07:58 UTC (permalink / raw) Dshash currently waits for lock unconditionally. It is inconvenient when we want to avoid being blocked by other processes. This commit adds alternative functions of dshash_find and dshash_find_or_insert that allows immediate return on lock failure. --- src/backend/lib/dshash.c | 98 +++++++++++++++++++++------------------- src/include/lib/dshash.h | 3 ++ 2 files changed, 55 insertions(+), 46 deletions(-) diff --git a/src/backend/lib/dshash.c b/src/backend/lib/dshash.c index 520bfa0979..853d78b528 100644 --- a/src/backend/lib/dshash.c +++ b/src/backend/lib/dshash.c @@ -383,6 +383,10 @@ dshash_get_hash_table_handle(dshash_table *hash_table) * the caller must take care to ensure that the entry is not left corrupted. * The lock mode is either shared or exclusive depending on 'exclusive'. * + * If found is not NULL, *found is set to true if the key is found in the hash + * table. If the key is not found, *found is set to false and a pointer to a + * newly created entry is returned. + * * The caller must not lock a lock already. * * Note that the lock held is in fact an LWLock, so interrupts will be held on @@ -392,36 +396,7 @@ dshash_get_hash_table_handle(dshash_table *hash_table) void * dshash_find(dshash_table *hash_table, const void *key, bool exclusive) { - dshash_hash hash; - size_t partition; - dshash_table_item *item; - - hash = hash_key(hash_table, key); - partition = PARTITION_FOR_HASH(hash); - - Assert(hash_table->control->magic == DSHASH_MAGIC); - Assert(!hash_table->find_locked); - - LWLockAcquire(PARTITION_LOCK(hash_table, partition), - exclusive ? LW_EXCLUSIVE : LW_SHARED); - ensure_valid_bucket_pointers(hash_table); - - /* Search the active bucket. */ - item = find_in_bucket(hash_table, key, BUCKET_FOR_HASH(hash_table, hash)); - - if (!item) - { - /* Not found. */ - LWLockRelease(PARTITION_LOCK(hash_table, partition)); - return NULL; - } - else - { - /* The caller will free the lock by calling dshash_release_lock. */ - hash_table->find_locked = true; - hash_table->find_exclusively_locked = exclusive; - return ENTRY_FROM_ITEM(item); - } + return dshash_find_extended(hash_table, key, exclusive, false, false, NULL); } /* @@ -439,31 +414,60 @@ dshash_find_or_insert(dshash_table *hash_table, const void *key, bool *found) { - dshash_hash hash; - size_t partition_index; - dshash_partition *partition; + return dshash_find_extended(hash_table, key, true, false, true, found); +} + + +/* + * Find the key in the hash table. + * + * "exclusive" is the lock mode in which the partition for the returned item + * is locked. If "nowait" is true, the function immediately returns if + * required lock was not acquired. "insert" indicates insert mode. In this + * mode new entry is inserted and set *found to false. *found is set to true if + * found. "found" must be non-null in this mode. + */ +void * +dshash_find_extended(dshash_table *hash_table, const void *key, + bool exclusive, bool nowait, bool insert, bool *found) +{ + dshash_hash hash = hash_key(hash_table, key); + size_t partidx = PARTITION_FOR_HASH(hash); + dshash_partition *partition = &hash_table->control->partitions[partidx]; + LWLockMode lockmode = exclusive ? LW_EXCLUSIVE : LW_SHARED; dshash_table_item *item; - hash = hash_key(hash_table, key); - partition_index = PARTITION_FOR_HASH(hash); - partition = &hash_table->control->partitions[partition_index]; - - Assert(hash_table->control->magic == DSHASH_MAGIC); - Assert(!hash_table->find_locked); + /* must be exclusive when insert allowed */ + Assert(!insert || (exclusive && found != NULL)); restart: - LWLockAcquire(PARTITION_LOCK(hash_table, partition_index), - LW_EXCLUSIVE); + if (!nowait) + LWLockAcquire(PARTITION_LOCK(hash_table, partidx), lockmode); + else if (!LWLockConditionalAcquire(PARTITION_LOCK(hash_table, partidx), + lockmode)) + return NULL; + ensure_valid_bucket_pointers(hash_table); /* Search the active bucket. */ item = find_in_bucket(hash_table, key, BUCKET_FOR_HASH(hash_table, hash)); if (item) - *found = true; + { + if (found) + *found = true; + } else { - *found = false; + if (found) + *found = false; + + if (!insert) + { + /* The caller didn't told to add a new entry. */ + LWLockRelease(PARTITION_LOCK(hash_table, partidx)); + return NULL; + } /* Check if we are getting too full. */ if (partition->count > MAX_COUNT_PER_PARTITION(hash_table)) @@ -479,7 +483,8 @@ restart: * Give up our existing lock first, because resizing needs to * reacquire all the locks in the right order to avoid deadlocks. */ - LWLockRelease(PARTITION_LOCK(hash_table, partition_index)); + LWLockRelease(PARTITION_LOCK(hash_table, partidx)); + resize(hash_table, hash_table->size_log2 + 1); goto restart; @@ -493,12 +498,13 @@ restart: ++partition->count; } - /* The caller must release the lock with dshash_release_lock. */ + /* The caller will free the lock by calling dshash_release_lock. */ hash_table->find_locked = true; - hash_table->find_exclusively_locked = true; + hash_table->find_exclusively_locked = exclusive; return ENTRY_FROM_ITEM(item); } + /* * Remove an entry by key. Returns true if the key was found and the * corresponding entry was removed. diff --git a/src/include/lib/dshash.h b/src/include/lib/dshash.h index a6ea377173..5b8114d041 100644 --- a/src/include/lib/dshash.h +++ b/src/include/lib/dshash.h @@ -91,6 +91,9 @@ extern void *dshash_find(dshash_table *hash_table, const void *key, bool exclusive); extern void *dshash_find_or_insert(dshash_table *hash_table, const void *key, bool *found); +extern void *dshash_find_extended(dshash_table *hash_table, const void *key, + bool exclusive, bool nowait, bool insert, + bool *found); extern bool dshash_delete_key(dshash_table *hash_table, const void *key); extern void dshash_delete_entry(dshash_table *hash_table, void *entry); extern void dshash_release_lock(dshash_table *hash_table, void *entry); -- 2.27.0 ----Next_Part(Fri_Mar__5_17_18_56_2021_497)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v48-0003-Make-archiver-process-an-auxiliary-process.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Fix some memory leaks in ecpg.addons @ 2023-11-08 17:01 Tristan Partin <[email protected]> 2023-11-08 17:07 ` Re: Fix some memory leaks in ecpg.addons Tom Lane <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Tristan Partin @ 2023-11-08 17:01 UTC (permalink / raw) To: pgsql-hackers clang and gcc both now support -fsanitize=address,undefined. These are really useful to me personally when trying to debug issues. Unfortunately ecpg code has a ton of memory leaks, which makes builds really painful. It would be great to fix all of them, but I don't have the patience to try to read flex/bison code. Here are two memory leak fixes in any case. -- Tristan Partin Neon (https://neon.tech) Attachments: [text/x-patch] v1-0001-Patch-two-memory-leaks-in-ecpg.addons.patch (1.5K, ../../[email protected]/2-v1-0001-Patch-two-memory-leaks-in-ecpg.addons.patch) download | inline diff: From af69e1711e87ae96d75814fc83a8588a4bdf7cac Mon Sep 17 00:00:00 2001 From: Tristan Partin <[email protected]> Date: Wed, 8 Nov 2023 10:53:02 -0600 Subject: [PATCH v1] Patch two memory leaks in ecpg.addons make_str() allocates memory, but we were never releasing it. There are more issues in this code, but these were just two easy ones to fix. --- src/interfaces/ecpg/preproc/ecpg.addons | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/interfaces/ecpg/preproc/ecpg.addons b/src/interfaces/ecpg/preproc/ecpg.addons index e94da2a3f8..dd1ea39c6e 100644 --- a/src/interfaces/ecpg/preproc/ecpg.addons +++ b/src/interfaces/ecpg/preproc/ecpg.addons @@ -51,6 +51,8 @@ ECPG: stmtExecuteStmt block str[strlen(str) - 1] = '\0'; sprintf(length, "%zu", strlen(str)); add_variable_to_tail(&argsinsert, new_variable(str, ECPGmake_simple_type(ECPGt_const, length, 0), 0), &no_indicator); + + free(str); } output_statement(cat_str(3, mm_strdup("execute"), mm_strdup("$0"), $1.type), 0, ECPGst_exec_with_exprlist); } @@ -79,6 +81,8 @@ ECPG: stmtPrepareStmt block str[strlen(str) - 1] = '\0'; sprintf(length, "%zu", strlen(str)); add_variable_to_tail(&argsinsert, new_variable(str, ECPGmake_simple_type(ECPGt_const, length, 0), 0), &no_indicator); + + free(str); } output_statement(cat_str(5, mm_strdup("prepare"), mm_strdup("$0"), $1.type, mm_strdup("as"), $1.stmt), 0, ECPGst_prepare); } -- Tristan Partin Neon (https://neon.tech) ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Fix some memory leaks in ecpg.addons 2023-11-08 17:01 Fix some memory leaks in ecpg.addons Tristan Partin <[email protected]> @ 2023-11-08 17:07 ` Tom Lane <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2023-11-08 17:07 UTC (permalink / raw) To: Tristan Partin <[email protected]>; +Cc: pgsql-hackers "Tristan Partin" <[email protected]> writes: > clang and gcc both now support -fsanitize=address,undefined. These are > really useful to me personally when trying to debug issues. > Unfortunately ecpg code has a ton of memory leaks, which makes builds > really painful. It would be great to fix all of them, but I don't have > the patience to try to read flex/bison code. Here are two memory leak > fixes in any case. I'm kind of failing to see the point. As you say, the ecpg preprocessor leaks memory like there's no tomorrow. But given its usage (process one source file and exit) I'm not sure that is worth much effort to fix. And what does it buy to fix just two spots? regards, tom lane ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-11-08 17:07 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-13 07:58 [PATCH v48 2/7] Add conditional lock feature to dshash Kyotaro Horiguchi <[email protected]> 2023-11-08 17:01 Fix some memory leaks in ecpg.addons Tristan Partin <[email protected]> 2023-11-08 17:07 ` Re: Fix some memory leaks in ecpg.addons Tom Lane <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox