fix: 修复多个严重问题并改进代码质量
1. 修复空指针 panic 风险 - serveSignal() 添加 g.Running nil 检查 2. 修复错误未检查 - startDaemon/startTask 添加 cmd.Start() 错误处理 3. 修复 Task PID 未写入 - startTask() 现在正确写入 taskPidFile 4. 添加 PID 文件清理 - 进程退出时自动删除 PID 文件 5. 修复 goroutine 泄漏 - task 分支添加 signal.Stop() 6. 解耦日志依赖 - 移除 gologger，改为 Logger 接口 7. 添加单元测试 - godaemon_test.go 覆盖核心功能 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
kingecg
parent
6baf99cd11
commit
06f75dbe60
102
godaemon.go
102
godaemon.go
|
|
@ -12,10 +12,22 @@ import (
|
|||
"strconv"
|
||||
"strings"
|
||||
"syscall"
|
||||
|
||||
"git.kingecg.top/kingecg/gologger"
|
||||
)
|
||||
|
||||
// Logger defines the logging interface used by GoDaemon
|
||||
type Logger interface {
|
||||
Debug(v ...any)
|
||||
Info(v ...any)
|
||||
Error(v ...any)
|
||||
}
|
||||
|
||||
// defaultLogger is a simple logger that uses fmt.Println
|
||||
type defaultLogger struct{}
|
||||
|
||||
func (d *defaultLogger) Debug(v ...any) { fmt.Println(v...) }
|
||||
func (d *defaultLogger) Info(v ...any) { fmt.Println(v...) }
|
||||
func (d *defaultLogger) Error(v ...any) { fmt.Print("[ERROR] "); fmt.Println(v...) }
|
||||
|
||||
// Constants defining environment variable keys and values used for process identification
|
||||
const (
|
||||
daemon_env_key = "_go_daemon" // Environment variable key for process role identification
|
||||
|
|
@ -26,13 +38,13 @@ const (
|
|||
|
||||
// GoDaemon represents a daemon process manager
|
||||
type GoDaemon struct {
|
||||
pidFile string // Path to file storing daemon process PID
|
||||
taskPidFile string // Path to file storing task process PID
|
||||
flag *string // Command line flag for signal control
|
||||
sigChan chan os.Signal // Channel for receiving OS signals
|
||||
state string // Current state of the daemon ("running", "stopped", etc.)
|
||||
*gologger.Logger // Embedded logger for logging
|
||||
Running *exec.Cmd // Currently running task process
|
||||
pidFile string // Path to file storing daemon process PID
|
||||
taskPidFile string // Path to file storing task process PID
|
||||
flag *string // Command line flag for signal control
|
||||
sigChan chan os.Signal // Channel for receiving OS signals
|
||||
state string // Current state of the daemon ("running", "stopped", etc.)
|
||||
logger Logger // Logger for logging
|
||||
Running *exec.Cmd // Currently running task process
|
||||
|
||||
StartFn func(*GoDaemon) // Function called when task starts
|
||||
StopFn func(*GoDaemon) // Function called when task stops
|
||||
|
|
@ -44,8 +56,11 @@ type GoDaemon struct {
|
|||
// - int: process ID if found and valid, 0 otherwise
|
||||
func (g *GoDaemon) GetPid() int {
|
||||
pids, ferr := os.ReadFile(g.pidFile)
|
||||
pid, err := strconv.Atoi(string(pids))
|
||||
if err != nil || ferr != nil {
|
||||
if ferr != nil {
|
||||
return 0
|
||||
}
|
||||
pid, err := strconv.Atoi(strings.TrimSpace(string(pids)))
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
return pid
|
||||
|
|
@ -57,8 +72,11 @@ func (g *GoDaemon) GetPid() int {
|
|||
// - int: process ID if found and valid, 0 otherwise
|
||||
func (g *GoDaemon) GetTaskPid() int {
|
||||
pids, ferr := os.ReadFile(g.taskPidFile)
|
||||
pid, err := strconv.Atoi(string(pids))
|
||||
if err != nil || ferr != nil {
|
||||
if ferr != nil {
|
||||
return 0
|
||||
}
|
||||
pid, err := strconv.Atoi(strings.TrimSpace(string(pids)))
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
return pid
|
||||
|
|
@ -97,7 +115,7 @@ func (g *GoDaemon) Start() {
|
|||
fmt.Println("Daemon process not found")
|
||||
return
|
||||
}
|
||||
g.Debug("Send signal:", p.Pid, sig)
|
||||
g.logger.Debug("Send signal:", p.Pid, sig)
|
||||
p.Signal(sig)
|
||||
} else if IsDaemon() {
|
||||
pid := os.Getpid()
|
||||
|
|
@ -110,22 +128,24 @@ func (g *GoDaemon) Start() {
|
|||
|
||||
for {
|
||||
|
||||
g.Debug("Starting new task")
|
||||
g.logger.Debug("Starting new task")
|
||||
g.Running = g.startTask()
|
||||
g.state = "running"
|
||||
g.Running.Process.Wait()
|
||||
if g.state == "stopped" {
|
||||
g.Debug("daemon is stopped, exit now")
|
||||
g.logger.Debug("daemon is stopped, exit now")
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
} else {
|
||||
waiter := make(chan os.Signal, 1)
|
||||
signal.Notify(waiter, syscall.SIGTERM, syscall.SIGHUP)
|
||||
g.StartFn(g)
|
||||
g.Info("daemon task is started")
|
||||
g.logger.Info("daemon task is started")
|
||||
<-waiter
|
||||
g.Info("daemon task will be stopped")
|
||||
g.logger.Info("daemon task will be stopped")
|
||||
signal.Stop(waiter)
|
||||
g.StopFn(g)
|
||||
}
|
||||
}
|
||||
|
|
@ -143,7 +163,21 @@ func (g *GoDaemon) serveSignal() {
|
|||
g.state = "restart"
|
||||
}
|
||||
|
||||
g.Running.Process.Signal(syscall.SIGTERM)
|
||||
if g.Running != nil && g.Running.Process != nil {
|
||||
g.Running.Process.Signal(syscall.SIGTERM)
|
||||
// Wait for task to exit after sending SIGTERM
|
||||
g.Running.Wait()
|
||||
}
|
||||
// Cleanup PID files when daemon stops
|
||||
if g.state == "stopped" {
|
||||
g.cleanupPIDFiles()
|
||||
}
|
||||
}
|
||||
|
||||
// cleanupPIDFiles removes the PID files on daemon exit
|
||||
func (g *GoDaemon) cleanupPIDFiles() {
|
||||
os.Remove(g.pidFile)
|
||||
os.Remove(g.taskPidFile)
|
||||
}
|
||||
|
||||
// getDaemonProcess retrieves the daemon process instance if it's running
|
||||
|
|
@ -157,7 +191,7 @@ func (g *GoDaemon) getDaemonProcess() *os.Process {
|
|||
}
|
||||
p, err := os.FindProcess(pid)
|
||||
if err != nil {
|
||||
g.Debug(err)
|
||||
g.logger.Debug(err)
|
||||
}
|
||||
serr := p.Signal(syscall.Signal(0))
|
||||
if serr != nil {
|
||||
|
|
@ -182,7 +216,11 @@ func (g *GoDaemon) startDaemon() {
|
|||
cmd.Env = append(cmd.Env, daemon_env_key+"="+daemon_process)
|
||||
pargs := os.Args[1:]
|
||||
cmd.Env = append(cmd.Env, daemon_taskargs+"="+strings.Join(pargs, ";"))
|
||||
cmd.Start()
|
||||
if err := cmd.Start(); err != nil {
|
||||
g.logger.Error("failed to start daemon:", err)
|
||||
return
|
||||
}
|
||||
fmt.Println("daemon started with pid:", cmd.Process.Pid)
|
||||
}
|
||||
|
||||
// startTask starts a new task process with the configured arguments
|
||||
|
|
@ -202,7 +240,12 @@ func (g *GoDaemon) startTask() *exec.Cmd {
|
|||
cmd.Stdout = os.Stdout
|
||||
cmd.Stderr = os.Stderr
|
||||
cmd.Env = append(cmd.Env, daemon_env_key+"="+daemon_task)
|
||||
cmd.Start()
|
||||
if err := cmd.Start(); err != nil {
|
||||
g.logger.Error("failed to start task:", err)
|
||||
return nil
|
||||
}
|
||||
// Write task PID to file
|
||||
os.WriteFile(g.taskPidFile, []byte(strconv.Itoa(cmd.Process.Pid)), 0644)
|
||||
return cmd
|
||||
}
|
||||
|
||||
|
|
@ -242,8 +285,21 @@ func IsDaemonTask() bool {
|
|||
// Returns:
|
||||
// - *GoDaemon: initialized GoDaemon instance
|
||||
func NewGoDaemon(start, stop func(*GoDaemon)) *GoDaemon {
|
||||
return NewGoDaemonWithLogger(start, stop, &defaultLogger{})
|
||||
}
|
||||
|
||||
// NewGoDaemonWithLogger creates a new GoDaemon instance with a custom logger
|
||||
//
|
||||
// Parameters:
|
||||
// - start: function to be called when the task process starts
|
||||
// - stop: function to be called when the task process stops
|
||||
// - logger: custom logger implementation
|
||||
//
|
||||
// Returns:
|
||||
// - *GoDaemon: initialized GoDaemon instance
|
||||
func NewGoDaemonWithLogger(start, stop func(*GoDaemon), logger Logger) *GoDaemon {
|
||||
godaemon := &GoDaemon{
|
||||
Logger: gologger.GetLogger("daemon"),
|
||||
logger: logger,
|
||||
}
|
||||
|
||||
execName, _ := os.Executable()
|
||||
|
|
|
|||
|
|
@ -0,0 +1,215 @@
|
|||
package godaemon
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// mockLogger is a simple mock logger for testing
|
||||
type mockLogger struct {
|
||||
debugLogs []any
|
||||
infoLogs []any
|
||||
errorLogs []any
|
||||
}
|
||||
|
||||
func (m *mockLogger) Debug(v ...any) { m.debugLogs = append(m.debugLogs, v) }
|
||||
func (m *mockLogger) Info(v ...any) { m.infoLogs = append(m.infoLogs, v) }
|
||||
func (m *mockLogger) Error(v ...any) { m.errorLogs = append(m.errorLogs, v) }
|
||||
|
||||
func TestIsMaster(t *testing.T) {
|
||||
// Clear the environment variable first
|
||||
os.Unsetenv(daemon_env_key)
|
||||
|
||||
if !IsMaster() {
|
||||
t.Error("IsMaster() should return true when environment variable is not set")
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsDaemon(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
envValue string
|
||||
expected bool
|
||||
}{
|
||||
{"daemon process", daemon_process, true},
|
||||
{"task process", daemon_task, false},
|
||||
{"empty", "", false},
|
||||
{"unknown value", "unknown", false},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if tt.envValue == "" {
|
||||
os.Unsetenv(daemon_env_key)
|
||||
} else {
|
||||
os.Setenv(daemon_env_key, tt.envValue)
|
||||
}
|
||||
defer os.Unsetenv(daemon_env_key)
|
||||
|
||||
if got := IsDaemon(); got != tt.expected {
|
||||
t.Errorf("IsDaemon() = %v, want %v", got, tt.expected)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsDaemonTask(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
envValue string
|
||||
expected bool
|
||||
}{
|
||||
{"task process", daemon_task, true},
|
||||
{"daemon process", daemon_process, false},
|
||||
{"empty", "", false},
|
||||
{"unknown value", "unknown", false},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if tt.envValue == "" {
|
||||
os.Unsetenv(daemon_env_key)
|
||||
} else {
|
||||
os.Setenv(daemon_env_key, tt.envValue)
|
||||
}
|
||||
defer os.Unsetenv(daemon_env_key)
|
||||
|
||||
if got := IsDaemonTask(); got != tt.expected {
|
||||
t.Errorf("IsDaemonTask() = %v, want %v", got, tt.expected)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewGoDaemon(t *testing.T) {
|
||||
logger := &mockLogger{}
|
||||
startCalled := false
|
||||
stopCalled := false
|
||||
|
||||
startFn := func(g *GoDaemon) { startCalled = true }
|
||||
stopFn := func(g *GoDaemon) { stopCalled = true }
|
||||
|
||||
gd := NewGoDaemonWithLogger(startFn, stopFn, logger)
|
||||
|
||||
if gd == nil {
|
||||
t.Fatal("NewGoDaemonWithLogger returned nil")
|
||||
}
|
||||
|
||||
if gd.StartFn == nil || gd.StopFn == nil {
|
||||
t.Error("StartFn or StopFn is nil")
|
||||
}
|
||||
|
||||
if gd.logger == nil {
|
||||
t.Error("logger is nil")
|
||||
}
|
||||
|
||||
if gd.pidFile == "" || gd.taskPidFile == "" {
|
||||
t.Error("PID file paths are not set")
|
||||
}
|
||||
|
||||
// Verify functions are properly assigned
|
||||
gd.StartFn(gd)
|
||||
if !startCalled {
|
||||
t.Error("StartFn was not called")
|
||||
}
|
||||
|
||||
gd.StopFn(gd)
|
||||
if !stopCalled {
|
||||
t.Error("StopFn was not called")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetPid(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
pidFile := filepath.Join(tmpDir, "test.pid")
|
||||
|
||||
gd := &GoDaemon{pidFile: pidFile}
|
||||
|
||||
// Test when file doesn't exist
|
||||
if got := gd.GetPid(); got != 0 {
|
||||
t.Errorf("GetPid() = %v, want 0 when file doesn't exist", got)
|
||||
}
|
||||
|
||||
// Test with invalid content
|
||||
os.WriteFile(pidFile, []byte("invalid"), 0644)
|
||||
if got := gd.GetPid(); got != 0 {
|
||||
t.Errorf("GetPid() = %v, want 0 for invalid content", got)
|
||||
}
|
||||
|
||||
// Test with valid PID
|
||||
os.WriteFile(pidFile, []byte("12345"), 0644)
|
||||
if got := gd.GetPid(); got != 12345 {
|
||||
t.Errorf("GetPid() = %v, want 12345", got)
|
||||
}
|
||||
|
||||
// Test with whitespace
|
||||
os.WriteFile(pidFile, []byte(" 67890 \n"), 0644)
|
||||
if got := gd.GetPid(); got != 67890 {
|
||||
t.Errorf("GetPid() = %v, want 67890 with whitespace", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetTaskPid(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
taskPidFile := filepath.Join(tmpDir, "test.task.pid")
|
||||
|
||||
gd := &GoDaemon{taskPidFile: taskPidFile}
|
||||
|
||||
// Test when file doesn't exist
|
||||
if got := gd.GetTaskPid(); got != 0 {
|
||||
t.Errorf("GetTaskPid() = %v, want 0 when file doesn't exist", got)
|
||||
}
|
||||
|
||||
// Test with valid PID
|
||||
os.WriteFile(taskPidFile, []byte("54321"), 0644)
|
||||
if got := gd.GetTaskPid(); got != 54321 {
|
||||
t.Errorf("GetTaskPid() = %v, want 54321", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCleanupPIDFiles(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
pidFile := filepath.Join(tmpDir, "test.pid")
|
||||
taskPidFile := filepath.Join(tmpDir, "test.task.pid")
|
||||
|
||||
gd := &GoDaemon{pidFile: pidFile, taskPidFile: taskPidFile}
|
||||
|
||||
// Create PID files
|
||||
os.WriteFile(pidFile, []byte("12345"), 0644)
|
||||
os.WriteFile(taskPidFile, []byte("67890"), 0644)
|
||||
|
||||
// Verify files exist
|
||||
if _, err := os.Stat(pidFile); os.IsNotExist(err) {
|
||||
t.Error("pidFile should exist before cleanup")
|
||||
}
|
||||
if _, err := os.Stat(taskPidFile); os.IsNotExist(err) {
|
||||
t.Error("taskPidFile should exist before cleanup")
|
||||
}
|
||||
|
||||
// Cleanup
|
||||
gd.cleanupPIDFiles()
|
||||
|
||||
// Verify files are removed
|
||||
if _, err := os.Stat(pidFile); !os.IsNotExist(err) {
|
||||
t.Error("pidFile should be removed after cleanup")
|
||||
}
|
||||
if _, err := os.Stat(taskPidFile); !os.IsNotExist(err) {
|
||||
t.Error("taskPidFile should be removed after cleanup")
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultLogger(t *testing.T) {
|
||||
logger := &defaultLogger{}
|
||||
|
||||
// These should not panic
|
||||
logger.Debug("debug", "message", 123)
|
||||
logger.Info("info", "message")
|
||||
logger.Error("error", "message")
|
||||
}
|
||||
|
||||
func TestLoggerInterface(t *testing.T) {
|
||||
// Verify that defaultLogger implements Logger interface
|
||||
var _ Logger = &defaultLogger{}
|
||||
var _ Logger = &mockLogger{}
|
||||
}
|
||||
Loading…
Reference in New Issue