tm: fix $rd reversion after chained async() resume#3821
Open
NormB wants to merge 1 commit intoOpenSIPS:masterfrom
Open
tm: fix $rd reversion after chained async() resume#3821NormB wants to merge 1 commit intoOpenSIPS:masterfrom
NormB wants to merge 1 commit intoOpenSIPS:masterfrom
Conversation
When async() is called from a resume route (chained async), the FL_TM_FAKE_REQ check in t_handle_async() prevents update_cloned_msg_from_msg() from saving message state to the transaction's shm clone. This causes $rd, $du, and other msg fields modified in the resume route to revert to their previous values when the next async operation completes through the reactor path. Remove the FL_TM_FAKE_REQ guard from the conditional. The update_cloned_msg_from_msg() function already handles faked_req sources correctly: it defers freeing old shm lumps (lines 1325-1336 of sip_msg.c), and free_faked_req() completes the deferred cleanup. Fixes OpenSIPS#3676
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
async()is called from a resume route (chained async), theFL_TM_FAKE_REQcheckin
t_handle_async()preventsupdate_cloned_msg_from_msg()from saving message stateto the transaction's shm clone. This causes
$rd,$du, and other msg fields modifiedin the resume route to revert to their previous values when the next async operation
completes through the reactor path.
The bug is intermittent because it only manifests when async goes through the reactor
(slow endpoint). When the transfer completes during curl's busy-wait loop (ASYNC_SYNC),
the resume route runs inline with the same pkg memory msg pointer and the bug is masked.
Fixes #3676
Root Cause
In
modules/tm/async.cline 446, the condition:The
FL_TM_FAKE_REQcheck was originally an optimization to avoid redundantly updatingthe shm clone from a message reconstructed from that same clone. However, when
async()is called from a resume route, the resume route's
route_typeis restored toREQUEST_ROUTE(viaswap_route_typeat line 194), but the msg is a faked_req withFL_TM_FAKE_REQset. The check blocks the update, so any$rd/$du/flag/lump changesmade in the resume route are lost when the next async completes through the reactor.
Fix
Remove the
FL_TM_FAKE_REQguard:This is safe because
update_cloned_msg_from_msg()already handles faked_req sourcescorrectly:
FL_TM_FAKE_REQis set(sip_msg.c:1325-1336), and
free_faked_req()completes the deferred free(t_msgbuilder.h:385-393)
original or faked_req sources
free_faked_reqonly frees lumps that differ from current clonefree_faked_req, not leakedThe only affected code path is
REQUEST_ROUTEwith a faked_req (chained async fromREQUEST_ROUTE). FAILURE_ROUTE, ONREPLY_ROUTE, BRANCH_ROUTE, LOCAL_ROUTE are unaffected.
Test Results
Tested on OpenSIPS 4.0.0-dev compiled from source.
Core tests:
$rdpreserved)$rdpreserved)Edge cases (all PASS):
$dupreservation across double async$rdchanges in single resume before next async$rdset back to original incoming value in resumeConcurrency stress: