Skip to content

Commit fccfddd

Browse files
author
David Grieser
committed
Added Job.HasStarted()
1 parent 5ca3f85 commit fccfddd

File tree

3 files changed

+27
-31
lines changed

3 files changed

+27
-31
lines changed

pkg/proc/basejob.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func (job *baseJob) SignalAll(sig syscall.Signal) {
3131
}
3232
}
3333

34-
if job.cmd == nil || job.cmd.Process == nil {
34+
if !job.HasStarted() {
3535
errFunc(
3636
fmt.Errorf("job is not running"),
3737
)
@@ -43,6 +43,9 @@ func (job *baseJob) SignalAll(sig syscall.Signal) {
4343
}
4444

4545
func (job *baseJob) signalAll(sig syscall.Signal) error {
46+
if !job.HasStarted() {
47+
return nil
48+
}
4649
if job.cmd.Process.Pid <= 0 { // Ensure PID is positive before negating
4750
return syscall.Errno(syscall.ESRCH) // No such process or invalid PID
4851
}
@@ -56,7 +59,7 @@ func (job *baseJob) Signal(sig os.Signal) {
5659
}
5760
}
5861

59-
if job.cmd == nil || job.cmd.Process == nil {
62+
if !job.HasStarted() {
6063
errFunc(
6164
fmt.Errorf("job is not running"),
6265
)
@@ -89,6 +92,10 @@ func (job *baseJob) IsControllable() bool {
8992
return job.Config.Controllable
9093
}
9194

95+
func (job *baseJob) HasStarted() bool {
96+
return job.cmd != nil && job.cmd.Process != nil
97+
}
98+
9299
func (job *baseJob) GetPhase() *JobPhase {
93100
return &job.phase
94101
}
@@ -183,13 +190,11 @@ func (job *baseJob) startOnce(ctx context.Context, process chan<- *os.Process) e
183190
select {
184191
// job errChan or failed
185192
case err := <-errChan:
186-
if job.cmd != nil && job.cmd.Process != nil { // Check if process actually started
187-
if termErr := job.signalAll(syscall.SIGTERM); termErr != nil {
188-
if e, ok := termErr.(syscall.Errno); ok && e == syscall.ESRCH {
189-
// ESRCH (Error No Such Process) is fine, means process group already gone
190-
} else {
191-
l.WithError(termErr).Error("failed to send SIGTERM to job's process group")
192-
}
193+
if termErr := job.signalAll(syscall.SIGTERM); termErr != nil {
194+
if e, ok := termErr.(syscall.Errno); ok && e == syscall.ESRCH {
195+
// ESRCH (Error No Such Process) is fine, means process group already gone
196+
} else {
197+
l.WithError(termErr).Error("failed to send SIGTERM to job's process group")
193198
}
194199
}
195200

@@ -209,7 +214,7 @@ func (job *baseJob) startOnce(ctx context.Context, process chan<- *os.Process) e
209214
}
210215
return err
211216
case <-ctx.Done():
212-
if job.cmd != nil && job.cmd.Process != nil { // Check if process actually started
217+
if job.HasStarted() {
213218
// ctx canceled, try to terminate job
214219
_ = job.signalAll(syscall.SIGTERM)
215220
l.WithField("job.name", job.Config.Name).Info("sent SIGTERM to job's process group on ctx.Done")
@@ -226,9 +231,8 @@ func (job *baseJob) startOnce(ctx context.Context, process chan<- *os.Process) e
226231
return err
227232
}
228233
}
229-
// If job.cmd or job.cmd.Process is nil, it means the process didn't start or already cleaned up.
230234
l.WithField("job.name", job.Config.Name).Info("context done, but process was not running or already cleaned up.")
231-
return ctx.Err() // Return context error
235+
return ctx.Err()
232236
}
233237
}
234238

pkg/proc/job_common.go

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func (job *CommonJob) Watch() {
177177
}
178178

179179
func (job *CommonJob) IsRunning() bool {
180-
if job.cmd == nil || job.cmd.Process == nil || job.cmd.Process.Pid <= 0 {
180+
if !job.HasStarted() || job.cmd.Process.Pid <= 0 {
181181
return false
182182
}
183183
err := job.signal(syscall.Signal(0))
@@ -186,31 +186,24 @@ func (job *CommonJob) IsRunning() bool {
186186

187187
func (job *CommonJob) Restart() {
188188
job.restart = true
189-
// Ensure job.cmd and job.cmd.Process are not nil before trying to signal
190-
if job.cmd != nil && job.cmd.Process != nil {
191-
job.SignalAll(syscall.SIGTERM)
192-
}
193-
if job.interrupt != nil { // Check if interrupt function is set
189+
job.SignalAll(syscall.SIGTERM)
190+
if job.interrupt != nil {
194191
job.interrupt()
195192
}
196193
}
197194

198195
func (job *CommonJob) Stop() {
199196
job.stop = true
200-
// Ensure job.cmd and job.cmd.Process are not nil before trying to signal
201-
if job.cmd != nil && job.cmd.Process != nil {
202-
job.SignalAll(syscall.SIGTERM)
203-
}
204-
if job.interrupt != nil { // Check if interrupt function is set
197+
job.SignalAll(syscall.SIGTERM)
198+
if job.interrupt != nil {
205199
job.interrupt()
206200
}
207201
}
208202

209203
func (job *CommonJob) Status() *CommonJobStatus {
210-
running := job.IsRunning()
204+
running := job.HasStarted() && job.IsRunning()
211205
var pid int
212-
// Ensure job.cmd and job.cmd.Process are not nil before trying to access Pid
213-
if running && job.cmd != nil && job.cmd.Process != nil {
206+
if running {
214207
pid = job.cmd.Process.Pid
215208
}
216209
return &CommonJobStatus{

pkg/proc/job_lazy.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,7 @@ func (job *LazyJob) startProcessReaper(ctx context.Context) {
125125
continue
126126
}
127127

128-
// Ensure job.cmd and job.cmd.Process are not nil before accessing PID
129-
if job.cmd == nil || job.cmd.Process == nil {
128+
if !job.HasStarted() {
130129
l.Warn("job.process is not nil, but job.cmd or job.cmd.Process is nil; skipping reap cycle")
131130
job.lazyStartLock.Unlock()
132131
continue
@@ -148,16 +147,16 @@ func (job *LazyJob) startProcessReaper(ctx context.Context) {
148147
case <-graceTimer.C:
149148
job.lazyStartLock.Lock()
150149
// Check if the process we sent SIGTERM to is still running
151-
if job.process != nil && job.cmd != nil && job.cmd.Process != nil && job.cmd.Process.Pid == pidToReap {
150+
if job.process != nil && job.HasStarted() && job.cmd.Process.Pid == pidToReap {
152151
l.Warnf("process PID %d did not exit after SIGTERM and grace period; sending SIGKILL", pidToReap)
153152
if err := job.signalAll(syscall.SIGKILL); err != nil {
154-
l.WithError(err).Errorf("failed to send SIGKILL to PID %d", pidToReap)
153+
l.WithError(err).Errorf("failed to send1 SIGKILL to PID %d", pidToReap)
155154
}
156155
} else if job.process != nil {
157156
// Process is not nil, but it's not the one we targeted.
158157
// This could happen if the job was quickly restarted.
159158
currentPid := -1
160-
if job.cmd != nil && job.cmd.Process != nil {
159+
if job.HasStarted() {
161160
currentPid = job.cmd.Process.Pid
162161
}
163162
l.Warnf("original process PID %d seems to have exited or changed; current PID is %d. Skipping SIGKILL.", pidToReap, currentPid)

0 commit comments

Comments
 (0)