Skip to content

build: finalize embedded python before exit #1707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cpunion wants to merge 3 commits into
base: main
Choose a base branch
from
Open

build: finalize embedded python before exit #1707

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17e4048
build: finalize embedded python before exit
cpunion Mar 12, 2026
ec38107
test: make python sample outputs deterministic
cpunion Mar 12, 2026
4369713
test: update cl/test snapshots
cpunion Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions internal/build/main_module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,10 @@ func genMainModule(ctx *context, rtPkgPath string, pkg *packages.Package, needRu
defineWeakNoArgStub(mainPkg, "syscall.init")

var pyInit llssa.Function
var pyFinalize llssa.Function
if needPyInit {
pyInit = declareNoArgFunc(mainPkg, "Py_Initialize")
pyFinalize = declareNoArgFunc(mainPkg, "Py_Finalize")
}

var rtInit llssa.Function
Expand All @@ -90,7 +92,7 @@ func genMainModule(ctx *context, rtPkgPath string, pkg *packages.Package, needRu
mainInit := declareNoArgFunc(mainPkg, pkg.PkgPath+".init")
mainMain := declareNoArgFunc(mainPkg, pkg.PkgPath+".main")

entryFn := defineEntryFunction(ctx, mainPkg, argcVar, argvVar, argvValueType, runtimeStub, mainInit, mainMain, pyInit, rtInit, abiInit)
entryFn := defineEntryFunction(ctx, mainPkg, argcVar, argvVar, argvValueType, runtimeStub, mainInit, mainMain, pyInit, pyFinalize, rtInit, abiInit)

if needStart(ctx) {
defineStart(mainPkg, entryFn, argvValueType)
Expand All @@ -106,7 +108,7 @@ func genMainModule(ctx *context, rtPkgPath string, pkg *packages.Package, needRu
// The entry stores argc/argv, optionally disables stdio buffering, runs
// initialization hooks (Python, runtime, package init), and finally calls
// main.main before returning 0.
Comment on lines 108 to 110
Copy link
Copy Markdown
Contributor

@xgopilot xgopilot bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc comment no longer reflects the full behavior — the function now also calls Py_Finalize() after main.main before returning 0.

Suggested change
// The entry stores argc/argv, optionally disables stdio buffering, runs
// initialization hooks (Python, runtime, package init), and finally calls
// main.main before returning 0.
// The entry stores argc/argv, optionally disables stdio buffering, runs
// initialization hooks (Python, runtime, package init), calls main.main,
// finalizes embedded Python if initialized, and returns 0.

func defineEntryFunction(ctx *context, pkg llssa.Package, argcVar, argvVar llssa.Global, argvType llssa.Type, runtimeStub, mainInit, mainMain llssa.Function, pyInit, rtInit, abiInit llssa.Function) llssa.Function {
func defineEntryFunction(ctx *context, pkg llssa.Package, argcVar, argvVar llssa.Global, argvType llssa.Type, runtimeStub, mainInit, mainMain llssa.Function, pyInit, pyFinalize, rtInit, abiInit llssa.Function) llssa.Function {
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This function now has 11 parameters, which can make it hard to read and maintain. Consider grouping the initialization and finalization functions into a struct to improve clarity.

For example:

type entryHooks struct {
    runtimeStub llssa.Function
    mainInit    llssa.Function
    mainMain    llssa.Function
    pyInit      llssa.Function
    pyFinalize  llssa.Function
    rtInit      llssa.Function
    abiInit     llssa.Function
}

func defineEntryFunction(ctx *context, pkg llssa.Package, argcVar, argvVar llssa.Global, argvType llssa.Type, hooks *entryHooks) llssa.Function {
    // ...
    if hooks.pyInit != nil {
        b.Call(hooks.pyInit.Expr)
    }
    // ...
}

This would make the function signature more manageable and easier to extend in the future.

prog := pkg.Prog
entryName := "main"
if !needStart(ctx) && isWasmTarget(ctx.buildConf.Goos) {
Expand Down Expand Up @@ -137,6 +139,9 @@ func defineEntryFunction(ctx *context, pkg llssa.Package, argcVar, argvVar llssa
b.Call(runtimeStub.Expr)
b.Call(mainInit.Expr)
b.Call(mainMain.Expr)
if pyFinalize != nil {
b.Call(pyFinalize.Expr)
}
b.Return(prog.IntVal(0, prog.Int32()))
return fn
}
Expand Down
1 change: 1 addition & 0 deletions internal/build/main_module_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func TestGenMainModuleExecutable(t *testing.T) {
checks := []string{
"define i32 @main(",
"call void @Py_Initialize()",
"call void @Py_Finalize()",
"call void @\"example.com/foo.init\"()",
"define weak void @_start()",
}
Comment on lines 37 to 43
Copy link
Copy Markdown
Contributor

@xgopilot xgopilot bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The test checks that each substring is present in the IR, but doesn't verify call ordering. An accidental swap (e.g., Py_Finalize before main.main) would still pass. Consider adding an index-based ordering check:

initIdx := strings.Index(ir, "call void @Py_Initialize()")
mainIdx := strings.Index(ir, `call void @"example.com/foo.main"()`)
finalIdx := strings.Index(ir, "call void @Py_Finalize()")
if !(initIdx < mainIdx && mainIdx < finalIdx) {
    t.Fatalf("incorrect call ordering: init@%d, main@%d, finalize@%d", initIdx, mainIdx, finalIdx)
}

Expand Down
Loading