From cf7bc0120a6d43fd7d18a433eeac5689391b85d5 Mon Sep 17 00:00:00 2001 From: Olaf Wintermann Date: Fri, 11 Apr 2025 21:36:40 +0200 Subject: [PATCH] update ucx and use temp mempool/cxMempoolTransfer for thread safety --- application/store.c | 12 ++--- ucx/buffer.c | 7 ++- ucx/cx/array_list.h | 2 +- ucx/cx/buffer.h | 5 +-- ucx/cx/common.h | 4 +- ucx/cx/linked_list.h | 4 +- ucx/cx/list.h | 1 + ucx/cx/mempool.h | 43 ++++++++++++++++++ ucx/cx/printf.h | 8 ++-- ucx/cx/properties.h | 8 ++-- ucx/cx/string.h | 8 ++-- ucx/linked_list.c | 2 + ucx/mempool.c | 104 ++++++++++++++++++++++++++++++++++++++----- ucx/printf.c | 34 ++++++++++++-- 14 files changed, 198 insertions(+), 44 deletions(-) diff --git a/application/store.c b/application/store.c index 1af0fc7..75be1a7 100644 --- a/application/store.c +++ b/application/store.c @@ -703,14 +703,13 @@ typedef struct LoadAttachmentsJob { static int qthr_load_attachments(LoadAttachmentsJob *job) { - //CxMempool *mp = cxMempoolCreateSimple(32); - //job->temp_mp = mp; - // TODO: use separate mempool and transfer it to the resource mempool + CxMempool *mp = cxMempoolCreateSimple(32); + job->temp_mp = mp; DBUQuery *q = connection->createQuery(connection, NULL); dbuQuerySetSQL(q, SQL_ATTACHMENTS_GET); dbuQuerySetParamInt64(q, 1, job->note->resource_id); - DBUObjectBuilder *builder = dbuObjectBuilder(attachments_class, q, job->note->model->note_allocator); // use mp mempool allocator + DBUObjectBuilder *builder = dbuObjectBuilder(attachments_class, q, mp->allocator); job->result = dbuObjectBuilderGetList(builder); if(!job->result) { job->error = 1; @@ -722,7 +721,10 @@ static int qthr_load_attachments(LoadAttachmentsJob *job) { static void uithr_load_attachments_finished(UiEvent *event, LoadAttachmentsJob *job) { if(job->result) { - job->note->attachments = job->result; // TODO: transfer temp_mp first + CxMempool *note_mp = job->note->model->note_allocator->data; + cxMempoolTransfer(job->temp_mp, note_mp); + cxMempoolFree(job->temp_mp); + job->note->attachments = job->result; } if(job->resultcb) { diff --git a/ucx/buffer.c b/ucx/buffer.c index 1a7713e..981b161 100644 --- a/ucx/buffer.c +++ b/ucx/buffer.c @@ -139,6 +139,7 @@ int cxBufferSeek( npos = 0; break; default: + errno = EINVAL; return -1; } @@ -399,10 +400,8 @@ int cxBufferPut( } int cxBufferTerminate(CxBuffer *buffer) { - bool success = 0 == cxBufferPut(buffer, 0); - if (success) { - buffer->pos--; - buffer->size--; + if (0 == cxBufferPut(buffer, 0)) { + buffer->size = buffer->pos - 1; return 0; } else { return -1; diff --git a/ucx/cx/array_list.h b/ucx/cx/array_list.h index 198407f..847540a 100644 --- a/ucx/cx/array_list.h +++ b/ucx/cx/array_list.h @@ -727,7 +727,7 @@ CxList *cxArrayListCreate( * * If @p elem_size is #CX_STORE_POINTERS, the created list stores pointers instead of * copies of the added elements and the compare function will be automatically set - * to cx_cmp_ptr(), if none is given. + * to cx_cmp_ptr(). * * @param elem_size (@c size_t) the size of each element in bytes * @param initial_capacity (@c size_t) the initial number of elements the array can store diff --git a/ucx/cx/buffer.h b/ucx/cx/buffer.h index e286832..4330580 100644 --- a/ucx/cx/buffer.h +++ b/ucx/cx/buffer.h @@ -674,11 +674,10 @@ int cxBufferPut( /** * Writes a terminating zero to a buffer at the current position. * - * On successful write, @em neither the position @em nor the size of the buffer is - * increased. + * If successful, sets the size to the current position and advances the position by one. * * The purpose of this function is to have the written data ready to be used as - * a C string. + * a C string with the buffer's size being the length of that string. * * @param buffer the buffer to write to * @return zero, if the terminator could be written, non-zero otherwise diff --git a/ucx/cx/common.h b/ucx/cx/common.h index 604ca5f..b54153d 100644 --- a/ucx/cx/common.h +++ b/ucx/cx/common.h @@ -46,7 +46,7 @@ * Repositories:
* https://sourceforge.net/p/ucx/code * - or - - * https://develop.uap-core.de/hg/ucx + * https://uap-core.de/hg/ucx *

* *

LICENCE

@@ -150,7 +150,7 @@ */ #define cx_attr_malloc __attribute__((__malloc__)) -#ifndef __clang__ +#if !defined(__clang__) && __GNUC__ >= 11 /** * The pointer returned by the attributed function is supposed to be freed * by @p freefunc. diff --git a/ucx/cx/linked_list.h b/ucx/cx/linked_list.h index fcdae74..94185d6 100644 --- a/ucx/cx/linked_list.h +++ b/ucx/cx/linked_list.h @@ -77,7 +77,7 @@ CxList *cxLinkedListCreate( * * If @p elem_size is #CX_STORE_POINTERS, the created list stores pointers instead of * copies of the added elements and the compare function will be automatically set - * to cx_cmp_ptr(), if none is given. + * to cx_cmp_ptr(). * * @param elem_size (@c size_t) the size of each element in bytes * @return (@c CxList*) the created list @@ -393,7 +393,7 @@ void cx_linked_list_insert_sorted_chain( * @li @p loc_next and @p loc_prev (ancestor node is determined by using the prev pointer, overall O(1) performance) * @li @p loc_next and @p begin (ancestor node is determined by list traversal, overall O(n) performance) * - * @remark The @c next and @c prev pointers of the removed node are not cleared by this function and may still be used + * @remark The @c next and @c prev pointers of the removed chain are not cleared by this function and may still be used * to traverse to a former adjacent node in the list, or within the chain. * * @param begin a pointer to the beginning node pointer (optional) diff --git a/ucx/cx/list.h b/ucx/cx/list.h index 2df6c50..a358d02 100644 --- a/ucx/cx/list.h +++ b/ucx/cx/list.h @@ -894,6 +894,7 @@ static inline size_t cxListFindRemove( */ cx_attr_nonnull static inline void cxListSort(CxList *list) { + if (list->collection.sorted) return; list->cl->sort(list); list->collection.sorted = true; } diff --git a/ucx/cx/mempool.h b/ucx/cx/mempool.h index 43b3b17..35bf34e 100644 --- a/ucx/cx/mempool.h +++ b/ucx/cx/mempool.h @@ -156,6 +156,49 @@ int cxMempoolRegister( cx_destructor_func destr ); +/** + * Transfers all the memory managed by one pool to another. + * + * The allocator of the source pool will also be transferred and registered with the destination pool + * and stays valid, as long as the destination pool is not destroyed. + * + * The source pool will get a completely new allocator and can be reused or destroyed afterward. + * + * @param source the pool to move the memory from + * @param dest the pool where to transfer the memory to + * @retval zero success + * @retval non-zero failure + */ +cx_attr_nonnull +cx_attr_export +int cxMempoolTransfer( + CxMempool *source, + CxMempool *dest +); + +/** + * Transfers an object from one pool to another. + * + * You can only transfer objects that have been allocated by this pool. + * Objects that have been registered with cxMempoolRegister() cannot be transferred this way. + * + * @attention If the object maintains a reference to the pool's allocator, + * you must make sure to update that reference to the allocator of the destination pool. + * + * @param source the pool to move the memory from + * @param dest the pool where to transfer the memory to + * @param obj pointer to the object that shall be transferred + * @retval zero success + * @retval non-zero failure or object was not found in the source pool + */ +cx_attr_nonnull +cx_attr_export +int cxMempoolTransferObject( + CxMempool *source, + CxMempool *dest, + const void *obj +); + #ifdef __cplusplus } // extern "C" #endif diff --git a/ucx/cx/printf.h b/ucx/cx/printf.h index f02a80f..4ffd05f 100644 --- a/ucx/cx/printf.h +++ b/ucx/cx/printf.h @@ -229,7 +229,7 @@ cx_attr_printf(4, 5) cx_attr_cstr_arg(4) cx_attr_export int cx_sprintf_a( - CxAllocator *alloc, + const CxAllocator *alloc, char **str, size_t *len, const char *fmt, @@ -274,7 +274,7 @@ cx_attr_access_rw(2) cx_attr_access_rw(3) cx_attr_export int cx_vsprintf_a( - CxAllocator *alloc, + const CxAllocator *alloc, char **str, size_t *len, const char *fmt, @@ -333,7 +333,7 @@ cx_attr_access_rw(3) cx_attr_access_rw(4) cx_attr_export int cx_sprintf_sa( - CxAllocator *alloc, + const CxAllocator *alloc, char *buf, size_t *len, char **str, @@ -388,7 +388,7 @@ cx_attr_nonnull cx_attr_cstr_arg(5) cx_attr_export int cx_vsprintf_sa( - CxAllocator *alloc, + const CxAllocator *alloc, char *buf, size_t *len, char **str, diff --git a/ucx/cx/properties.h b/ucx/cx/properties.h index 619229f..2942738 100644 --- a/ucx/cx/properties.h +++ b/ucx/cx/properties.h @@ -551,10 +551,12 @@ CxPropertiesStatus cxPropertiesNext( /** * Creates a properties sink for an UCX map. * - * The values stored in the map will be pointers to strings allocated - * by #cx_strdup_a(). + * The values stored in the map will be pointers to freshly allocated, + * zero-terminated C strings (@c char*), which means the @p map should have been + * created with #CX_STORE_POINTERS. + * * The default stdlib allocator will be used, unless you specify a custom - * allocator in the optional @c data of the sink. + * allocator in the optional @c data field of the returned sink. * * @param map the map that shall consume the k/v-pairs. * @return the sink diff --git a/ucx/cx/string.h b/ucx/cx/string.h index 64c8dba..fee35d2 100644 --- a/ucx/cx/string.h +++ b/ucx/cx/string.h @@ -151,7 +151,7 @@ extern "C" { * * @param literal the string literal */ -#define CX_STR(literal) (cxstring){literal, sizeof(literal) - 1} +#define CX_STR(literal) ((cxstring){literal, sizeof(literal) - 1}) #endif @@ -879,7 +879,7 @@ cxmutstr cx_strdup_a_( * @see cx_strfree_a() */ #define cx_strdup_a(allocator, string) \ - cx_strdup_a_((allocator), cx_strcast((string))) + cx_strdup_a_((allocator), cx_strcast(string)) /** * Creates a duplicate of the specified string. @@ -894,7 +894,7 @@ cxmutstr cx_strdup_a_( * @see cx_strdup_a() * @see cx_strfree() */ -#define cx_strdup(string) cx_strdup_a_(cxDefaultAllocator, string) +#define cx_strdup(string) cx_strdup_a(cxDefaultAllocator, string) /** * Omits leading and trailing spaces. @@ -1091,7 +1091,7 @@ CxStrtokCtx cx_strtok_( * @return (@c CxStrtokCtx) a new string tokenization context */ #define cx_strtok(str, delim, limit) \ - cx_strtok_(cx_strcast((str)), cx_strcast((delim)), (limit)) + cx_strtok_(cx_strcast(str), cx_strcast(delim), (limit)) /** * Returns the next token. diff --git a/ucx/linked_list.c b/ucx/linked_list.c index 3833d44..66cf543 100644 --- a/ucx/linked_list.c +++ b/ucx/linked_list.c @@ -920,6 +920,8 @@ static size_t cx_ll_find_remove( const void *elem, bool remove ) { + if (list->collection.size == 0) return 0; + size_t index; cx_linked_list *ll = ((cx_linked_list *) list); cx_linked_list_node *node = cx_linked_list_find( diff --git a/ucx/mempool.c b/ucx/mempool.c index fea4d70..1080c55 100644 --- a/ucx/mempool.c +++ b/ucx/mempool.c @@ -38,24 +38,34 @@ struct cx_mempool_memory_s { char c[]; }; +static int cx_mempool_ensure_capacity( + struct cx_mempool_s *pool, + size_t needed_capacity +) { + if (needed_capacity <= pool->capacity) return 0; + size_t newcap = pool->capacity >= 1000 ? + pool->capacity + 1000 : pool->capacity * 2; + size_t newmsize; + if (pool->capacity > newcap || cx_szmul(newcap, + sizeof(struct cx_mempool_memory_s*), &newmsize)) { + errno = EOVERFLOW; + return 1; + } + struct cx_mempool_memory_s **newdata = realloc(pool->data, newmsize); + if (newdata == NULL) return 1; + pool->data = newdata; + pool->capacity = newcap; + return 0; +} + static void *cx_mempool_malloc( void *p, size_t n ) { struct cx_mempool_s *pool = p; - if (pool->size >= pool->capacity) { - size_t newcap = pool->capacity - (pool->capacity % 16) + 16; - size_t newmsize; - if (pool->capacity > newcap || cx_szmul(newcap, - sizeof(struct cx_mempool_memory_s*), &newmsize)) { - errno = EOVERFLOW; - return NULL; - } - struct cx_mempool_memory_s **newdata = realloc(pool->data, newmsize); - if (newdata == NULL) return NULL; - pool->data = newdata; - pool->capacity = newcap; + if (cx_mempool_ensure_capacity(pool, pool->size + 1)) { + return NULL; } struct cx_mempool_memory_s *mem = malloc(sizeof(cx_destructor_func) + n); @@ -235,3 +245,73 @@ CxMempool *cxMempoolCreate( return pool; } + +int cxMempoolTransfer( + CxMempool *source, + CxMempool *dest +) { + // safety check + if (source == dest) return 1; + + // ensure enough capacity in the destination pool + if (cx_mempool_ensure_capacity(dest, dest->size + source->size + 1)) { + return 1; // LCOV_EXCL_LINE + } + + // allocate a replacement allocator for the source pool + CxAllocator *new_source_allocator = malloc(sizeof(CxAllocator)); + if (new_source_allocator == NULL) { // LCOV_EXCL_START + return 1; + } // LCOV_EXCL_STOP + new_source_allocator->cl = &cx_mempool_allocator_class; + new_source_allocator->data = source; + + // transfer all the data + memcpy(&dest->data[dest->size], source->data, sizeof(source->data[0])*source->size); + dest->size += source->size; + + // register the old allocator with the new pool + // we have to remove const-ness for this, but that's okay here + CxAllocator *transferred_allocator = (CxAllocator*) source->allocator; + transferred_allocator->data = dest; + cxMempoolRegister(dest, transferred_allocator, free); + + // prepare the source pool for re-use + source->allocator = new_source_allocator; + memset(source->data, 0, source->size * sizeof(source->data[0])); + source->size = 0; + + return 0; +} + +int cxMempoolTransferObject( + CxMempool *source, + CxMempool *dest, + const void *obj +) { + // safety check + if (source == dest) return 1; + + // first, make sure that the dest pool can take the object + if (cx_mempool_ensure_capacity(dest, dest->size + 1)) { + return 1; // LCOV_EXCL_LINE + } + // search for the object + for (size_t i = 0; i < source->size; i++) { + struct cx_mempool_memory_s *mem = source->data[i]; + if (mem->c == obj) { + // remove from the source pool + size_t last_index = source->size - 1; + if (i != last_index) { + source->data[i] = source->data[last_index]; + source->data[last_index] = NULL; + } + source->size--; + // add to the target pool + dest->data[dest->size++] = mem; + return 0; + } + } + // not found + return 1; +} diff --git a/ucx/printf.c b/ucx/printf.c index bf691cf..a699a0f 100644 --- a/ucx/printf.c +++ b/ucx/printf.c @@ -132,7 +132,13 @@ cxmutstr cx_vasprintf_a( return s; } -int cx_sprintf_a(CxAllocator *alloc, char **str, size_t *len, const char *fmt, ... ) { +int cx_sprintf_a( + const CxAllocator *alloc, + char **str, + size_t *len, + const char *fmt, + ... +) { va_list ap; va_start(ap, fmt); int ret = cx_vsprintf_a(alloc, str, len, fmt, ap); @@ -140,7 +146,13 @@ int cx_sprintf_a(CxAllocator *alloc, char **str, size_t *len, const char *fmt, . return ret; } -int cx_vsprintf_a(CxAllocator *alloc, char **str, size_t *len, const char *fmt, va_list ap) { +int cx_vsprintf_a( + const CxAllocator *alloc, + char **str, + size_t *len, + const char *fmt, + va_list ap +) { va_list ap2; va_copy(ap2, ap); int ret = vsnprintf(*str, *len, fmt, ap); @@ -162,7 +174,14 @@ int cx_vsprintf_a(CxAllocator *alloc, char **str, size_t *len, const char *fmt, return ret; } -int cx_sprintf_sa(CxAllocator *alloc, char *buf, size_t *len, char **str, const char *fmt, ... ) { +int cx_sprintf_sa( + const CxAllocator *alloc, + char *buf, + size_t *len, + char **str, + const char *fmt, + ... +) { va_list ap; va_start(ap, fmt); int ret = cx_vsprintf_sa(alloc, buf, len, str, fmt, ap); @@ -170,7 +189,14 @@ int cx_sprintf_sa(CxAllocator *alloc, char *buf, size_t *len, char **str, const return ret; } -int cx_vsprintf_sa(CxAllocator *alloc, char *buf, size_t *len, char **str, const char *fmt, va_list ap) { +int cx_vsprintf_sa( + const CxAllocator *alloc, + char *buf, + size_t *len, + char **str, + const char *fmt, + va_list ap +) { va_list ap2; va_copy(ap2, ap); int ret = vsnprintf(buf, *len, fmt, ap); -- 2.43.5