]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
Forward cancellations to children, and don't cancel the parent until the child is...
authorArran Cudbard-Bell <a.cudbardb@freeradius.org>
Mon, 5 May 2025 19:50:12 +0000 (13:50 -0600)
committerArran Cudbard-Bell <a.cudbardb@freeradius.org>
Tue, 6 May 2025 17:56:00 +0000 (11:56 -0600)
src/lib/unlang/interpret.c
src/lib/unlang/subrequest.c
src/lib/unlang/unlang_priv.h

index a62fdb65258eb296f57129252934e35c96337f53..54996684299b1bd1532ff108778c1886f478bbdb 100644 (file)
@@ -1083,6 +1083,11 @@ void unlang_stack_signal(request_t *request, fr_signal_t action, int limit)
         *      There may be multiple resumption points in the
         *      stack, as modules can push xlats and function
         *      calls.
+        *
+        *      Note: Slightly confusingly, a cancellation signal
+        *      can still be delivered to a frame that is not
+        *      cancellable, but the frame won't be automatically
+        *      unwound.
         */
        for (i = depth; i >= limit; i--) {
                frame = &stack->frame[i];
@@ -1130,6 +1135,8 @@ void unlang_interpret_signal(request_t *request, fr_signal_t action)
 
        switch (action) {
        case FR_SIGNAL_CANCEL:
+       {
+               unlang_stack_frame_t *frame = &stack->frame[stack->depth];
                /*
                 *      Let anything that cares, know that the
                 *      request was forcefully stopped.
@@ -1138,10 +1145,15 @@ void unlang_interpret_signal(request_t *request, fr_signal_t action)
 
                /*
                 *      If the request is yielded, mark it as runnable
+                *
+                *      If the request was _not_ cancelled, it means
+                *      it's not cancellable, and we need to let the
+                *      request progress normally.
                 */
-               if (stack && is_yielded(&stack->frame[stack->depth]) && !unlang_request_is_scheduled(request)) {
+               if (stack && is_yielded(frame) && is_cancelled(frame) && !unlang_request_is_scheduled(request)) {
                        unlang_interpret_mark_runnable(request);
                }
+       }
                break;
 
        case FR_SIGNAL_DETACH:
index d537340ff82b055f47dc29aa1869c03b3eb100c3..efee68db4f815c2d977805bb6b233bf2d40a6082 100644 (file)
@@ -299,7 +299,18 @@ int unlang_subrequest_op_init(void)
                                .name = "subrequest",
                                .interpret = unlang_subrequest_parent_init,
                                .signal = unlang_subrequest_signal_child,
-                               .flag = UNLANG_OP_FLAG_DEBUG_BRACES | UNLANG_OP_FLAG_RCODE_SET,
+                               /*
+                                *      Frame can't be cancelled, because children need to
+                                *      write out status to the parent.  If we don't do this,
+                                *      then all children must be detachable and must detach
+                                *      so they don't try and write out status to a "done"
+                                *      parent.
+                                *
+                                *      It's easier to allow the child/parent relationship
+                                *      to end normally so that non-detachable requests are
+                                *      guaranteed the parent still exists.
+                                */
+                               .flag = UNLANG_OP_FLAG_DEBUG_BRACES | UNLANG_OP_FLAG_RCODE_SET | UNLANG_OP_FLAG_NO_CANCEL,
                                .frame_state_size = sizeof(unlang_frame_state_subrequest_t),
                                .frame_state_type = "unlang_frame_state_subrequest_t",
                        });
index 4ef4fcdaffd31fd3855d75354b9ef5810bbacb82..892dfe312150da5e450ccb3b82097e6f68012c95 100644 (file)
@@ -216,6 +216,11 @@ typedef enum CC_HINT(flag_enum) {
        UNLANG_OP_FLAG_DEBUG_BRACES     = 0x01,                 //!< Print debug braces.
        UNLANG_OP_FLAG_RCODE_SET        = 0x02,                 //!< Set request->rcode to the result of this operation.
        UNLANG_OP_FLAG_NO_CANCEL        = 0x04,                 //!< Must not be cancelled.
+                                                               ///< @Note Slightly confusingly, a cancellation signal
+                                                               ///< can still be delivered to a frame that is not
+                                                               ///< cancellable, but the frame won't be automatically
+                                                               ///< unwound.  This lets the frame know that cancellation
+                                                               ///< is desired, but can be ignored.
        UNLANG_OP_FLAG_BREAK_POINT      = 0x08,                 //!< Break point.
        UNLANG_OP_FLAG_RETURN_POINT     = 0x10,                 //!< Return point.
        UNLANG_OP_FLAG_CONTINUE_POINT   = 0x20                  //!< Continue point.