zhangyue-hashdata commented on code in PR #1322:
URL: https://github.com/apache/cloudberry/pull/1322#discussion_r2292796504
##########
src/backend/commands/resgroupcmds.c:
##########
@@ -1093,11 +1093,26 @@ alterResgroupCallback(XactEvent event, void *arg)
if (event == XACT_EVENT_COMMIT)
ResGroupAlterOnCommit(callbackCtx);
+ /*
+ * Free io_limit resources allocated in AlterResourceGroup().
+ *
+ * We need to handle two cases:
+ * 1. caps.io_limit != oldCaps.io_limit: case
RESGROUP_LIMIT_TYPE_IO_LIMIT
+ * 2. caps.io_limit == oldCaps.io_limit: other cases
+ *
+ * The pointer comparison (oldCaps.io_limit != caps.io_limit) is
crucial to
+ * avoid double free errors. When "other cases", both pointers might
+ * reference the same memory location, so we only free oldCaps.io_limit
if
+ * it's different from caps.io_limit.
+ */
if (callbackCtx->caps.io_limit != NIL)
+ {
cgroupOpsRoutine->freeio(callbackCtx->caps.io_limit);
- if (callbackCtx->caps.io_limit != NIL)
- cgroupOpsRoutine->freeio(callbackCtx->oldCaps.io_limit);
+ if (callbackCtx->oldCaps.io_limit != NIL &&
Review Comment:
According to the implementation of AlterResourceGroup(), the condition
`caps.io_limit == NULL && oldCaps.io_limit != NULL` should never occur. An
`Assert(!(caps.io_limit == NULL && oldCaps.io_limit != NULL))` is appropriate
here to verify this condition.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
