fix(python apps): fork child + exec appcmd in child to avoid issues by sicoyle · Pull Request #1580 · dapr/cli

sicoyle · 2026-01-29T21:32:10Z

Description

Fork is unsafe after threading has been used, at least for python. This is a known issue in the python community. See:

Forking after threading has been used can cause deadlocks/hangs when using threading primitives like Lock, Condition, or Queue in a process created via fork. All of this occurs within the internals of durabletask-python. This means it will impact any app using the python sdk, be it dapr agents or general python workflows. I tried various python versions and they all experience hangs, at various points due to this.

This PR corrects this by using syscall.ForkExec to fork a child, then execs python or whatever app language within that child so the app language starts via exec, avoiding it knowing about the fork. I will say a downfall of this is I lose the blue APP prefix stuff on the log lines. So this means changing the UX via terminals unfortunately. I tried adding that back with pipes, but then adding the pipes to give me the log prefix then broke my python app again causing the hang.

This is what it looks like btw with this PR. Blue INFO is from the sidecar and the app logs are in white without the ===APP=== prefix that we used to have:

I need to test this on a windows machine to make sure I don't leave zombies, and also I think at this point this is good enough to fix other OS's. I could leave a note in the docs that running python apps on windows could lead to unpredictable behavior for now and create a follow up issue for that. Thoughts?

Issue reference

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Extended the documentation

…problems Signed-off-by: Samantha Coyle <sam@diagrid.io>

Signed-off-by: Samantha Coyle <sam@diagrid.io>

…into fix-python-hangs

Signed-off-by: Samantha Coyle <sam@diagrid.io>

JoshVanL · 2026-02-04T10:27:20Z

@sicoyle tests are very red

Signed-off-by: Samantha Coyle <sam@diagrid.io>

…into fix-python-hangs

Signed-off-by: Samantha Coyle <sam@diagrid.io>

…into fix-python-hangs

Signed-off-by: Samantha Coyle <sam@diagrid.io>

codecov · 2026-02-10T22:15:27Z

Codecov Report

❌ Patch coverage is 0% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 15.45%. Comparing base (9780aab) to head (fd34ce5).
⚠️ Report is 19 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/standalone/run.go	0.00%	17 Missing ⚠️
pkg/standalone/stop.go	0.00%	10 Missing ⚠️
pkg/standalone/process_group_unix.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1580      +/-   ##
==========================================
- Coverage   15.74%   15.45%   -0.29%     
==========================================
  Files          64       64              
  Lines        7090     7220     +130     
==========================================
  Hits         1116     1116              
- Misses       5888     6018     +130     
  Partials       86       86

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Samantha Coyle <sam@diagrid.io>

pkg/standalone/stop.go

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T18:38:17Z

tests/e2e/standalone/uninstall_test.go

 	}

-	cli, err := dockerClient.NewClientWithOpts(dockerClient.FromEnv)
+	cli, err := dockerClient.NewClientWithOpts(dockerClient.FromEnv, dockerClient.WithVersion("1.48"))


Adding a hardcoded Docker API version "1.48" may cause compatibility issues with older Docker installations. The Docker client should ideally negotiate the API version automatically, or this version should be made configurable. Consider whether this version requirement is documented and whether it's the minimum supported version for all features used in these tests.

Suggested change

cli, err := dockerClient.NewClientWithOpts(dockerClient.FromEnv, dockerClient.WithVersion("1.48"))

cli, err := dockerClient.NewClientWithOpts(dockerClient.FromEnv)

I added this as I was getting a compatibility issue due to versions so pinned on one that would work.

cmd/run_windows.go

cmd/run.go

cmd/run_unix.go

Signed-off-by: Samantha Coyle <sam@diagrid.io>

sicoyle · 2026-02-23T23:39:43Z

ready again @JoshVanL pls

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cmd/run.go

pkg/standalone/run.go

cmd/run.go

pkg/standalone/stop.go

pkg/standalone/run.go

Signed-off-by: Samantha Coyle <sam@diagrid.io>

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cmd/run.go

Copilot · 2026-02-24T16:59:21Z

pkg/standalone/stop.go

+			err = syscall.Kill(-pgid, syscall.SIGTERM)
+			if err != nil {
+				// If process group doesn't exist (ESRCH), treat it as already stopped.
+				if err == syscall.ESRCH {
+					return nil
+				}
+				errKill := syscall.Kill(-pgid, syscall.SIGKILL)
+				// If process group doesn't exist, treat it as already stopped.
+				if errKill != nil && errKill != syscall.ESRCH {
+					return errKill
+				}
+			}
+			return nil


After sending SIGTERM (line 80), if it succeeds, the function returns immediately without waiting for the process to actually exit. This means the caller has no guarantee that the process group has terminated. Consider documenting that this is a fire-and-forget termination, or implementing a wait-and-verify mechanism if process termination confirmation is needed.

Copilot · 2026-02-24T16:59:22Z

pkg/standalone/stop.go

+				}
+				return nil
 			}
 			// Kill the whole process group.


Changed signal from SIGINT to SIGTERM, which is more conventional for graceful shutdown. However, this could potentially affect applications that specifically handle SIGINT differently than SIGTERM. Consider documenting this change in user-facing documentation or release notes to inform users about the signal change.

Suggested change

// Kill the whole process group.

// Kill the whole process group.

// Use SIGTERM for graceful shutdown of the process group. This differs from using SIGINT

// and may affect applications that handle SIGINT and SIGTERM differently.

Copilot · 2026-02-24T16:59:22Z

cmd/run_unix.go

+		return fmt.Errorf("failed to fork/exec app: %w", err)
+	}
+	if output.AppCMD == nil {
+		return fmt.Errorf("app command is nil")


After successfully calling ForkExec and before assigning the process, there's a check for output.AppCMD == nil. If this is nil, the function returns an error but the forked child process has already been started (via ForkExec at line 59) and is now orphaned. Consider either ensuring AppCMD is never nil before calling ForkExec, or cleaning up the forked process if AppCMD is unexpectedly nil by calling syscall.Kill on the returned pid.

Suggested change

return fmt.Errorf("app command is nil")

// Clean up the forked child process to avoid leaving it orphaned.

if killErr := syscall.Kill(pid, syscall.SIGKILL); killErr != nil {

return fmt.Errorf("app command is nil; failed to kill child process %d: %w", pid, killErr)

}

return fmt.Errorf("app command is nil; killed child process %d", pid)

cmd/run_unix.go

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix(python apps): fork child + exec appcmd in child to avoid issues

sicoyle added 2 commits January 29, 2026 14:49

fix(python apps): fork a child and exec appcmd in the child to avoid …

a64679c

…problems Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: final clean testing with pipes for app prefix

480a884

Signed-off-by: Samantha Coyle <sam@diagrid.io>

sicoyle requested review from a team as code owners January 29, 2026 21:32

sicoyle added 7 commits January 29, 2026 15:32

Merge branch 'master' into fix-python-hangs

6c802c1

style: appease linter

2492ef4

Signed-off-by: Samantha Coyle <sam@diagrid.io>

Merge branch 'fix-python-hangs' of ssh://github.com/sicoyle/dapr-cli …

9526f90

…into fix-python-hangs

fix: make windows tests happy

27e160a

Signed-off-by: Samantha Coyle <sam@diagrid.io>

style: appease linter

c51c42f

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: check for windows

647c648

Signed-off-by: Samantha Coyle <sam@diagrid.io>

style: appease linter again

9af9922

Signed-off-by: Samantha Coyle <sam@diagrid.io>

JoshVanL and others added 14 commits February 10, 2026 17:07

Merge branch 'master' into fix-python-hangs

def81e4

fix(build): update test

8e8516b

Signed-off-by: Samantha Coyle <sam@diagrid.io>

Merge branch 'fix-python-hangs' of ssh://github.com/sicoyle/dapr-cli …

f81bea0

…into fix-python-hangs

Merge branch 'master' into fix-python-hangs

5b2eb25

fix: updates for build to pass

5e599f9

Signed-off-by: Samantha Coyle <sam@diagrid.io>

Merge branch 'fix-python-hangs' of ssh://github.com/sicoyle/dapr-cli …

efe4d8a

…into fix-python-hangs

fix: ensure successful status on exit

e39d3a0

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix(build): use more build tags to prevent windows os build failures

3335080

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: use os specific cmds

3bc794c

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: updates for build

d4f9047

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: updates for build again

a9b896e

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: fixes for e2e tests

f8460a7

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: final fixes for build i think

877699a

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix(tests): last update for tests

167668f

Signed-off-by: Samantha Coyle <sam@diagrid.io>

sicoyle added 3 commits February 10, 2026 16:17

fix: add check to prevent unrelated panic

a3abbd8

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: updates for e2e tests to be happy

b2cae33

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: acct for diff responses in assert

08ae06a

Signed-off-by: Samantha Coyle <sam@diagrid.io>

sicoyle added 2 commits February 23, 2026 11:22

fix: pin docker version to fix unrelated testissues

9f4e89a

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: allow time for port releases

2c217a7

Signed-off-by: Samantha Coyle <sam@diagrid.io>

JoshVanL requested a review from Copilot February 23, 2026 18:32

Copilot started reviewing on behalf of JoshVanL February 23, 2026 18:33 View session

JoshVanL requested changes Feb 23, 2026

View reviewed changes

pkg/standalone/stop.go Outdated Show resolved Hide resolved

pkg/standalone/stop.go Outdated Show resolved Hide resolved

Copilot AI reviewed Feb 23, 2026

View reviewed changes

sicoyle added 6 commits February 23, 2026 13:09

fix: address feedback

5b36220

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: see if this helps with the e2e faliures

2f65eef

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: up the times

73bf225

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: up timeouts

e96aebc

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: handle scheudler port conflict issue flake

c91b919

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: update to fix flakey scheduler test

e1a10e8

Signed-off-by: Samantha Coyle <sam@diagrid.io>

acroca requested a review from Copilot February 24, 2026 10:15

Copilot started reviewing on behalf of acroca February 24, 2026 10:15 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

cmd/run.go Show resolved Hide resolved

cmd/run.go Outdated Show resolved Hide resolved

pkg/standalone/run.go Outdated Show resolved Hide resolved

cmd/run.go Outdated Show resolved Hide resolved

acroca reviewed Feb 24, 2026

View reviewed changes

pkg/standalone/stop.go Outdated Show resolved Hide resolved

pkg/standalone/run.go Show resolved Hide resolved

sicoyle added 2 commits February 24, 2026 10:25

style: updates per feedback

9ee93d4

Signed-off-by: Samantha Coyle <sam@diagrid.io>

fix: update for test

1c4af6c

Signed-off-by: Samantha Coyle <sam@diagrid.io>

JoshVanL requested a review from Copilot February 24, 2026 16:51

Copilot started reviewing on behalf of JoshVanL February 24, 2026 16:52 View session

cicoyle previously approved these changes Feb 24, 2026

View reviewed changes

Copilot AI reviewed Feb 24, 2026

View reviewed changes

fix: address final feedback

88883d9

Signed-off-by: Samantha Coyle <sam@diagrid.io>

sicoyle dismissed cicoyle’s stale review via 88883d9 February 24, 2026 17:06

JoshVanL approved these changes Feb 24, 2026

View reviewed changes

cicoyle merged commit 26d9692 into dapr:master Feb 24, 2026
35 of 38 checks passed

sicoyle deleted the fix-python-hangs branch February 24, 2026 20:35

sicoyle pushed a commit to sicoyle/dapr-cli that referenced this pull request Feb 24, 2026

Merge pull request dapr#1580 from sicoyle/fix-python-hangs

4208c1b

fix(python apps): fork child + exec appcmd in child to avoid issues

sicoyle mentioned this pull request Feb 24, 2026

fix(python apps): fork child + exec appcmd in child to avoid issues #1595

Merged

3 tasks

	cli, err := dockerClient.NewClientWithOpts(dockerClient.FromEnv, dockerClient.WithVersion("1.48"))
	cli, err := dockerClient.NewClientWithOpts(dockerClient.FromEnv)

-		return fmt.Errorf("app command is nil")
+		// Clean up the forked child process to avoid leaving it orphaned.
+		if killErr := syscall.Kill(pid, syscall.SIGKILL); killErr != nil {
+			return fmt.Errorf("app command is nil; failed to kill child process %d: %w", pid, killErr)
+		}
+		return fmt.Errorf("app command is nil; killed child process %d", pid)

Conversation

sicoyle commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue reference

Checklist

Uh oh!

JoshVanL commented Feb 4, 2026

Uh oh!

codecov bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

sicoyle Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sicoyle commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sicoyle commented Jan 29, 2026 •

edited

Loading

codecov bot commented Feb 10, 2026 •

edited

Loading