From 6bb12a07a7e2cbab572511772d1eb3cd902d7ed0 Mon Sep 17 00:00:00 2001 From: Matthieu Dorier Date: Wed, 26 Nov 2025 16:22:17 +0000 Subject: [PATCH 1/4] documented memory ownership of the raft_event structure passed to raft_step --- include/raft.h.in | 147 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) diff --git a/include/raft.h.in b/include/raft.h.in index 831200ff..9980be6c 100644 --- a/include/raft.h.in +++ b/include/raft.h.in @@ -1022,6 +1022,153 @@ RAFT_API void raft_seed(struct raft *r, unsigned random); * The @update output parameter contains changes that user code should perform * after the raft_step() call returns (e.g. new entries that must be persisted, * new messages that must be sent, etc.). + * + * Memory Ownership Model for raft_event Fields + * ============================================= + * + * The caller must understand the memory ownership semantics for each event type + * to avoid memory leaks or use-after-free bugs. This section documents who owns + * the memory for each field in the raft_event union after raft_step() returns. + * + * RAFT_START Event + * ---------------- + * - event->start.term: Scalar value (no ownership concerns) + * - event->start.voted_for: Scalar value (no ownership concerns) + * - event->start.metadata: PARTIAL OWNERSHIP TRANSFER. The nested configuration + * field is transferred to raft (moved into r->configuration). The metadata + * structure itself is NOT freed by raft and must be freed by the caller after + * raft_step() returns, regardless of success or error. The caller must NOT + * close the configuration field after the call, as it has been transferred. + * - event->start.start_index: Scalar value (no ownership concerns) + * - event->start.entries: OWNERSHIP TRANSFERRED to raft. On error, raft_step() + * automatically frees the entries via entryBatchesDestroy(). On success, the + * entries are consumed. The caller must NOT free the entries after calling + * raft_step(), regardless of success or error. The entries must be allocated + * such that all entries in a batch share the same batch pointer, and each + * distinct batch pointer appears only once in the entries array for proper + * cleanup. + * - event->start.n_entries: Scalar value (no ownership concerns) + * + * RAFT_RECEIVE Event + * ------------------ + * - event->receive.message: The message structure itself is NOT owned by raft + * (caller retains ownership), EXCEPT for the following nested fields which ARE + * owned by raft after raft_step() is called, REGARDLESS of success or error: + * + * For RAFT_APPEND_ENTRIES messages: + * - message->append_entries.entries: OWNERSHIP TRANSFERRED immediately upon + * calling raft_step(). The entries array and the batch memory referenced by + * entries[].batch are owned by raft after the call. The caller must NOT free + * this memory, even if raft_step() returns an error. + * + * For RAFT_INSTALL_SNAPSHOT messages: + * - message->install_snapshot.data.base: OWNERSHIP TRANSFERRED immediately upon + * calling raft_step(). The snapshot data buffer is owned by raft after the + * call. The caller must NOT free this memory, even if raft_step() returns an + * error. + * - message->install_snapshot.conf: OWNERSHIP TRANSFERRED immediately upon + * calling raft_step(). The configuration is owned by raft after the call. + * The caller must NOT close this configuration, even if raft_step() returns + * an error. + * + * All other message types: No nested ownership transfer; the message content + * is copied or consumed during processing. + * + * IMPORTANT: The ownership transfer happens immediately when raft_step() is + * called, not based on the result. This means the caller must NEVER attempt to + * free or clean up these transferred fields, regardless of whether raft_step() + * succeeds or fails. + * + * RAFT_PERSISTED_ENTRIES Event + * ---------------------------- + * - event->persisted_entries.index: Scalar value (no ownership concerns) + * + * RAFT_PERSISTED_SNAPSHOT Event + * ----------------------------- + * - event->persisted_snapshot.metadata: Passed by value; the structure is copied. + * The nested configuration is copied internally. Caller retains ownership of + * the event structure. + * - event->persisted_snapshot.offset: Scalar value (no ownership concerns) + * - event->persisted_snapshot.last: Scalar value (no ownership concerns) + * + * RAFT_CONFIGURATION Event + * ------------------------ + * - event->configuration.index: Scalar value (no ownership concerns) + * - event->configuration.conf: OWNERSHIP TRANSFERRED to raft. The configuration + * will be consumed if needed or closed if not needed. The caller must NOT close + * this configuration after calling raft_step(), regardless of success or error. + * + * RAFT_SNAPSHOT Event + * ------------------- + * - event->snapshot.metadata: Passed by value; the structure is copied. The + * nested configuration is NOT deeply copied - the caller must ensure the + * configuration remains valid during the raft_step() call, but may free it + * afterward. Caller retains ownership. + * - event->snapshot.trailing: Scalar value (no ownership concerns) + * + * RAFT_TIMEOUT Event + * ------------------ + * No event-specific fields (no ownership concerns) + * + * RAFT_SUBMIT Event + * ----------------- + * - event->submit.entries: OWNERSHIP TRANSFERRED to raft on success. On error, + * the caller must free the entries. The entries must have their batch field + * properly set (non-NULL) as raft expects batched entries. Each entry's buf.base + * must point into the batch memory. On error, the caller is responsible for + * freeing both the entries array and the referenced batches. + * - event->submit.n: Scalar value (no ownership concerns) + * + * RAFT_CATCH_UP Event + * ------------------- + * - event->catch_up.server_id: Scalar value (no ownership concerns) + * + * RAFT_TRANSFER Event + * ------------------- + * - event->transfer.server_id: Scalar value (no ownership concerns) + * + * Error Handling and Memory Cleanup + * ================================== + * + * When raft_step() returns an error: + * - For RAFT_START events: The caller must free the metadata structure itself + * (but NOT the configuration field, which has been transferred). The entries + * are automatically freed by raft_step() on error, so the caller must NOT + * free them. + * - For RAFT_SUBMIT events: The caller must clean up entries using + * entryBatchesDestroy() or equivalent. + * - For RAFT_RECEIVE events: Ownership of transferred nested fields (entries, + * snapshot data, configurations) has ALREADY been transferred even on error, + * so the caller must NOT free them. + * - For RAFT_CONFIGURATION events: Ownership has been transferred; the caller + * must NOT close the configuration. + * - For all other events: No cleanup required by caller. + * + * The caller should check the return value and handle errors appropriately: + * + * int rv = raft_step(r, &event, &update); + * if (rv != 0) { + * if (event.type == RAFT_START) { + * // Clean up metadata structure (but not the configuration!) + * if (event.start.metadata != NULL) { + * raft_free(event.start.metadata); + * } + * // DO NOT free entries - raft_step already freed them on error + * } else if (event.type == RAFT_SUBMIT) { + * // Clean up entries on submit error + * entryBatchesDestroy(event.submit.entries, event.submit.n); + * } + * // Handle other error cases... + * } else { + * // On success + * if (event.type == RAFT_START) { + * // Still need to free the metadata structure itself + * if (event.start.metadata != NULL) { + * raft_free(event.start.metadata); + * } + * // DO NOT free entries or configuration + * } + * } */ RAFT_API int raft_step(struct raft *r, struct raft_event *event, From dff384e90d2064f137b3e1697a9ce6964df2be17 Mon Sep 17 00:00:00 2001 From: Matthieu Dorier Date: Mon, 1 Dec 2025 09:51:01 +0000 Subject: [PATCH 2/4] changed memory ownership documentation for RAFT_START event --- include/raft.h.in | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/include/raft.h.in b/include/raft.h.in index 9980be6c..9a070084 100644 --- a/include/raft.h.in +++ b/include/raft.h.in @@ -1034,19 +1034,24 @@ RAFT_API void raft_seed(struct raft *r, unsigned random); * ---------------- * - event->start.term: Scalar value (no ownership concerns) * - event->start.voted_for: Scalar value (no ownership concerns) - * - event->start.metadata: PARTIAL OWNERSHIP TRANSFER. The nested configuration - * field is transferred to raft (moved into r->configuration). The metadata - * structure itself is NOT freed by raft and must be freed by the caller after - * raft_step() returns, regardless of success or error. The caller must NOT - * close the configuration field after the call, as it has been transferred. + * - event->start.metadata: Ownership fully remains with the caller, who can + * free it (and any nested field) after the call to raft_step. * - event->start.start_index: Scalar value (no ownership concerns) - * - event->start.entries: OWNERSHIP TRANSFERRED to raft. On error, raft_step() - * automatically frees the entries via entryBatchesDestroy(). On success, the - * entries are consumed. The caller must NOT free the entries after calling - * raft_step(), regardless of success or error. The entries must be allocated - * such that all entries in a batch share the same batch pointer, and each - * distinct batch pointer appears only once in the entries array for proper - * cleanup. + * - event->start.entries: PARTIAL OWNERSHIP TRANSFERRED. The ownership of the + * event->start.entries pointer always remains with the caller. On success, + * the entries themselves are consumed by the RAFT instance and must not be + * freed. On error, ownership of the individual entries remain with the + * caller. The entries must be allocated with raft_malloc (or another raft_* + * allocation functions) such that all entries in a batch share the same + * batch pointer, and each distinct batch pointer appears only once in the + * entries array for proper cleanup. For instance if the entries array is + * made of 4 entries, entries[0].batch should be set to the allocated memory + * where the 4 entries are located, while entries[1,2,3].batch should be set + * to NULL. entries[*].buf.base should point to memory inside the batch. + * If successfully consumed by RAFT, raft_free will be called on + * entries[0].batch when RAFT no longer needs these entries to be kept in + * memory. Hence the caller must NOT free the entries after a successful call + * to raft_step(). * - event->start.n_entries: Scalar value (no ownership concerns) * * RAFT_RECEIVE Event From 4d49611f29a7c80b11a45b44bf98374829821765 Mon Sep 17 00:00:00 2001 From: Matthieu Dorier Date: Wed, 3 Dec 2025 10:45:30 +0000 Subject: [PATCH 3/4] updated documentation about RAFT_START and RAFT_RECEIVE --- include/raft.h.in | 45 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/include/raft.h.in b/include/raft.h.in index 9a070084..7e681136 100644 --- a/include/raft.h.in +++ b/include/raft.h.in @@ -1041,17 +1041,32 @@ RAFT_API void raft_seed(struct raft *r, unsigned random); * event->start.entries pointer always remains with the caller. On success, * the entries themselves are consumed by the RAFT instance and must not be * freed. On error, ownership of the individual entries remain with the - * caller. The entries must be allocated with raft_malloc (or another raft_* - * allocation functions) such that all entries in a batch share the same - * batch pointer, and each distinct batch pointer appears only once in the - * entries array for proper cleanup. For instance if the entries array is - * made of 4 entries, entries[0].batch should be set to the allocated memory - * where the 4 entries are located, while entries[1,2,3].batch should be set - * to NULL. entries[*].buf.base should point to memory inside the batch. - * If successfully consumed by RAFT, raft_free will be called on - * entries[0].batch when RAFT no longer needs these entries to be kept in - * memory. Hence the caller must NOT free the entries after a successful call - * to raft_step(). + * caller. If successfully consumed by RAFT, raft_free will be called on + * entries[x].batch once per unique batch when RAFT no longer needs these + * entries to be kept in memory. Hence the caller must NOT free the entries + * after a successful call to raft_step(). + * + * The entries must be allocated with raft_malloc (or another raft_* + * allocation functions). The batch pointer of each entry must point to the + * base address of the allocated memory where the entry is located. As an + * example, with 7 entries placed into two batches, the batch pointers may + * look like the following: + * + * entries[0].batch = ; // Allocation for the first 4 entries + * entries[1].batch = ; + * entries[2].batch = ; + * entries[3].batch = ; + * entries[4].batch = ; // Allocation for the last 3 entries + * entries[5].batch = ; + * entries[6].batch = ; + * + * Importantly, batch locations cannot alternate, e.g. the following would be + * invalid: + * + * entries[0].batch = ; + * entries[1].batch = ; + * entries[2].batch = ; + * * - event->start.n_entries: Scalar value (no ownership concerns) * * RAFT_RECEIVE Event @@ -1066,6 +1081,14 @@ RAFT_API void raft_seed(struct raft *r, unsigned random); * entries[].batch are owned by raft after the call. The caller must NOT free * this memory, even if raft_step() returns an error. * + * IMPORTANT: Contrary to RAFT_START, RAFT_RECEIVE expects all the entries + * to belong to a SINGLE batch, so all the entries' batch pointer must be + * the same. This is because we expect RAFT_RECEIVE events to come from the + * network and hence to have been placed in a single allocation. The batch + * pointer does not need to point to the start of the first entry's data, + * it simply must point to the address to pass to raft_free when it RAFT + * will deallocate it. + * * For RAFT_INSTALL_SNAPSHOT messages: * - message->install_snapshot.data.base: OWNERSHIP TRANSFERRED immediately upon * calling raft_step(). The snapshot data buffer is owned by raft after the From aee66da809dd39f6590c9dc1e2cd605c35c91203 Mon Sep 17 00:00:00 2001 From: Matthieu Dorier Date: Wed, 3 Dec 2025 13:08:48 +0000 Subject: [PATCH 4/4] changed documentation for RECEIVE --- include/raft.h.in | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/include/raft.h.in b/include/raft.h.in index 7e681136..fbd52dfb 100644 --- a/include/raft.h.in +++ b/include/raft.h.in @@ -1076,10 +1076,12 @@ RAFT_API void raft_seed(struct raft *r, unsigned random); * owned by raft after raft_step() is called, REGARDLESS of success or error: * * For RAFT_APPEND_ENTRIES messages: - * - message->append_entries.entries: OWNERSHIP TRANSFERRED immediately upon - * calling raft_step(). The entries array and the batch memory referenced by - * entries[].batch are owned by raft after the call. The caller must NOT free - * this memory, even if raft_step() returns an error. + * - message->append_entries.entries: OWNERSHIP PARTIALLY TRANSFERRED upon + * calling raft_step(). The caller retains ownership of the entries array. + * The batch memory referenced by entries[].batch is owned by raft after + * the call. The caller must NOT free this memory, even if raft_step() + * succeeds. The ownership of the entries' memory is however left to the + * caller in case of a failure of raft_step(). * * IMPORTANT: Contrary to RAFT_START, RAFT_RECEIVE expects all the entries * to belong to a SINGLE batch, so all the entries' batch pointer must be @@ -1090,14 +1092,11 @@ RAFT_API void raft_seed(struct raft *r, unsigned random); * will deallocate it. * * For RAFT_INSTALL_SNAPSHOT messages: - * - message->install_snapshot.data.base: OWNERSHIP TRANSFERRED immediately upon - * calling raft_step(). The snapshot data buffer is owned by raft after the - * call. The caller must NOT free this memory, even if raft_step() returns an - * error. - * - message->install_snapshot.conf: OWNERSHIP TRANSFERRED immediately upon - * calling raft_step(). The configuration is owned by raft after the call. - * The caller must NOT close this configuration, even if raft_step() returns - * an error. + * - message->install_snapshot.data.base: OWNERSHIP TRANSFERRED, the snapshot + * data buffer is owned by raft after a successful call to raft_step. The + * caller must NOT free this memory, unless raft_step returns an error, in + * which case the caller retains ownership of the memory. + * - message->install_snapshot.conf: ownership is retained by the caller. * * All other message types: No nested ownership transfer; the message content * is copied or consumed during processing.