Skip to content

Commit 522c01c

Browse files
committed
mem: run the pkg-arena swap-in after free_route_lists(), not before
The previous commit (e0c63d2f45) gave each forked child its own independent HG_MALLOC pkg reservation instead of COW-inheriting the parent's, but placed the swap-in too early: internal_fork_child_setup() (the default post-fork handler) runs free_route_lists() afterwards for process types that do not need the parsed script, pkg_free()ing route AST nodes that were allocated by the PARENT before fork. Those pointers only make sense against the ORIGINAL (COW-inherited) arena. With mem_block already swapped to the fresh arena by that point, free_route_lists() ended up pkg_free()ing pointers from the old arena through the new arena's bookkeeping - a cross-arena mismatch that corrupts the new arena's free-list at best. In practice it surfaced as a clean SIGBUS instead: cell_set_next() writes directly through the freed pointer, which lands in the old, still-COW arena; if the hugetlb pool is low by the time that page-fault happens (as it will be, once enough earlier siblings have each taken their own 16-32MB reservation), the kernel has nothing left to back the COW copy and the process dies mid-startup. Reproduced live on .244 (49 of 53 processes got tier-1 MAP_HUGETLB, the pool ran dry for the last 4, and the process immediately after - TCP main - crashed inside free_route_lists() via this exact path: internal_fork -> internal_fork_child_setup -> free_route_lists -> free_action_list -> free_expr -> hg_cell_free -> cell_set_next SIGBUS). Fix: move the pkg-arena swap-in to after the post-fork handler chain runs, so free_route_lists() still operates against the still-correct original arena and completes its cleanup there before it is abandoned. Nothing between the old and new call sites (process-table bookkeeping, seed_child) touches pkg memory, and the only other handler in that chain (the profiler's post-fork hook) doesn't either - verified by reading pt.c's _fork_handlers list directly rather than assuming. Verified: standalone incremental rebuild (make opensips) compiles and links clean with no warnings.
1 parent ec9f829 commit 522c01c

1 file changed

Lines changed: 50 additions & 35 deletions

File tree

‎pt.c‎

Lines changed: 50 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -645,41 +645,6 @@ int internal_fork(const struct internal_fork_params *ifpp)
645645
hg_malloc_child_init((struct hg_block *)shm_dbg_block);
646646
#endif
647647
}
648-
#ifdef PKG_MALLOC
649-
if (mem_allocator_pkg == MM_HG_MALLOC ||
650-
mem_allocator_pkg == MM_HG_MALLOC_DBG) {
651-
/*
652-
* pkg memory is private (MAP_PRIVATE), not shared like shm -
653-
* so unlike the shm case above, resetting just the fast-path
654-
* bookkeeping on the COW-inherited block is not enough: the
655-
* block's own metadata (chunk list, bump offset, lock) lives
656-
* INSIDE that same COW-shared region, so every process's
657-
* writes to it silently diverge into disconnected private
658-
* copies while all still pulling physical hugepages from the
659-
* ONE shared reservation the parent made pre-fork - with
660-
* enough worker processes this exhausts the hugetlb pool well
661-
* beyond what pkg_mem_size alone would suggest, and confuses
662-
* the kernel's own hugetlb reservation accounting.
663-
*
664-
* Give this child its own independent reservation instead of
665-
* inheriting the parent's: hg_malloc_init() already knows how
666-
* to try tier 1 (MAP_HUGETLB) and gracefully fall back through
667-
* the tier ladder if the pool doesn't have enough left for
668-
* this specific process - exactly the semantics wanted here,
669-
* just never previously invoked per child. The parent's own
670-
* inherited mapping is simply left unmapped-but-unreferenced;
671-
* this child never touches it again.
672-
*/
673-
struct hg_block *child_pkg =
674-
hg_malloc_init(pkg_mem_size, "pkg", 0);
675-
if (!child_pkg) {
676-
LM_CRIT("failed to init this child's own pkg memory "
677-
"(%lu bytes)\n", pkg_mem_size);
678-
exit(-1);
679-
}
680-
mem_block = child_pkg;
681-
}
682-
#endif
683648
#endif /* HG_MALLOC */
684649

685650
/* set uid */
@@ -715,6 +680,56 @@ int internal_fork(const struct internal_fork_params *ifpp)
715680
child_startup_failed();
716681
}
717682
}
683+
#ifdef PKG_MALLOC
684+
if (mem_allocator_pkg == MM_HG_MALLOC ||
685+
mem_allocator_pkg == MM_HG_MALLOC_DBG) {
686+
/*
687+
* pkg memory is private (MAP_PRIVATE), not shared like shm - so
688+
* unlike the shm case above, resetting just the fast-path
689+
* bookkeeping on the COW-inherited block is not enough: the
690+
* block's own metadata (chunk list, bump offset, lock) lives
691+
* INSIDE that same COW-shared region, so every process's
692+
* writes to it silently diverge into disconnected private
693+
* copies while all still pulling physical hugepages from the
694+
* ONE shared reservation the parent made pre-fork - with
695+
* enough worker processes this exhausts the hugetlb pool well
696+
* beyond what pkg_mem_size alone would suggest, and confuses
697+
* the kernel's own hugetlb reservation accounting.
698+
*
699+
* Give this child its own independent reservation instead of
700+
* inheriting the parent's: hg_malloc_init() already knows how
701+
* to try tier 1 (MAP_HUGETLB) and gracefully fall back through
702+
* the tier ladder if the pool doesn't have enough left for this
703+
* specific process - exactly the semantics wanted here, just
704+
* never previously invoked per child.
705+
*
706+
* MUST run after the post-fork handler chain below, not before:
707+
* internal_fork_child_setup() (the default handler) calls
708+
* free_route_lists(), which pkg_frees the route AST this child
709+
* inherited from the parent pre-fork. Those pointers only make
710+
* sense against the ORIGINAL (COW-inherited) arena. Swapping
711+
* mem_block to a fresh arena before that handler runs made it
712+
* free foreign pointers through the new arena's bookkeeping -
713+
* silent cross-arena corruption at best, and a SIGBUS at worst
714+
* (the COW page-fault into the abandoned old arena has nothing
715+
* left to fall back on once the hugetlb pool is tight, which is
716+
* exactly what exposed this). Running the swap here instead
717+
* lets free_route_lists() finish its cleanup against the still-
718+
* correct original arena first; only then is it abandoned - the
719+
* parent's inherited mapping is left unmapped-but-unreferenced,
720+
* and this child never touches it again.
721+
*/
722+
struct hg_block *child_pkg =
723+
hg_malloc_init(pkg_mem_size, "pkg", 0);
724+
if (!child_pkg) {
725+
LM_CRIT("failed to init this child's own pkg memory "
726+
"(%lu bytes)\n", pkg_mem_size);
727+
exit(-1);
728+
}
729+
mem_block = child_pkg;
730+
}
731+
#endif
732+
718733
atomic_store(&pt[process_no].startup_result, CHLD_OK);
719734
return 0;
720735
}else{

0 commit comments

Comments
 (0)