Skip to content

Commit 114c0a0

Browse files
author
root
committed
host: fix flynn run --enable-log hang caused by OOM logger WaitGroup leak
The OOM notification logger shared the container's MuxConfig (same JobID), causing Follow() to add a 4th entry to the job's WaitGroup. Since cleanup() only closes the 3 tracked stdio streams, the WaitGroup never reached zero, blocking jobDoneCh() forever and preventing AttachExit from being sent. Fix by giving the OOM logger a separate config with a distinct JobID so it gets its own independent WaitGroup. Also fix the Attach deferred func to handle the case where the container exits before client is set, and add a jobDones map to logmux so late subscribers can detect already-finished jobs. Fixes: TestRun, TestRunSignal, TestLog, TestLogFilter, TestLogFollow, TestLogStderr (all were hanging indefinitely).
1 parent df22756 commit 114c0a0

3 files changed

Lines changed: 31 additions & 7 deletions

File tree

controller/client/v1/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ func (c *Client) StreamJobEvents(appID string, output chan *ct.Job) (stream.Stre
624624
}
625625

626626
func (c *Client) WatchJobEvents(appID, releaseID string) (ct.JobWatcher, error) {
627-
events := make(chan *ct.Job)
627+
events := make(chan *ct.Job, 100)
628628
stream, err := c.StreamJobEvents(appID, events)
629629
if err != nil {
630630
return nil, err

host/libcontainer_backend.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,7 +1161,11 @@ func (c *Container) watch(ready chan<- error, buffer host.LogBuffer) error {
11611161
log.Warn("error subscribing to OOM notifications (non-fatal)", "err", err)
11621162
} else {
11631163
go func() {
1164-
logger := c.l.LogMux.Logger(logagg.MsgIDInit, c.MuxConfig, "component", "flynn-host")
1164+
// Use a separate config so the OOM logger's Follow() does not
1165+
// add to the job's WaitGroup (which tracks stdio stream closure).
1166+
oomConfig := *c.MuxConfig
1167+
oomConfig.JobID = c.job.ID + ".oom"
1168+
logger := c.l.LogMux.Logger(logagg.MsgIDInit, &oomConfig, "component", "flynn-host")
11651169
defer logger.Close()
11661170
for range notifyOOM {
11671171
logger.Crit("FATAL: a container process was killed due to lack of available memory")
@@ -1397,14 +1401,21 @@ func (l *LibcontainerBackend) Attach(req *AttachRequest) (err error) {
13971401
}
13981402

13991403
defer func() {
1400-
if client != nil && (req.Job.Job.Config.TTY || req.Stream) && err == io.EOF {
1401-
<-client.done
1404+
if (req.Job.Job.Config.TTY || req.Stream) && err == io.EOF {
1405+
if client != nil {
1406+
<-client.done
1407+
}
14021408
job := l.State.GetJob(req.Job.Job.ID)
1409+
if job == nil {
1410+
return
1411+
}
14031412
if job.Status == host.StatusDone || job.Status == host.StatusCrashed {
14041413
err = ExitError(*job.ExitStatus)
14051414
return
14061415
}
1407-
err = errors.New(*job.Error)
1416+
if job.Error != nil {
1417+
err = errors.New(*job.Error)
1418+
}
14081419
}
14091420
}()
14101421

@@ -1485,8 +1496,10 @@ func (l *LibcontainerBackend) Attach(req *AttachRequest) (err error) {
14851496
}
14861497

14871498
ch := make(chan *rfc5424.Message)
1499+
l.Logger.Info("streaming log for attached job", "fn", "Attach", "job.id", req.Job.Job.ID, "logs", req.Logs, "stream", req.Stream)
14881500
stream, err := l.LogMux.StreamLog(req.Job.Job.Metadata["flynn-controller.app"], req.Job.Job.ID, req.Logs, req.Stream, ch)
14891501
if err != nil {
1502+
l.Logger.Error("StreamLog error", "fn", "Attach", "job.id", req.Job.Job.ID, "err", err)
14901503
return err
14911504
}
14921505
defer stream.Close()
@@ -1508,6 +1521,7 @@ func (l *LibcontainerBackend) Attach(req *AttachRequest) (err error) {
15081521
return nil
15091522
}
15101523
}
1524+
l.Logger.Info("log stream ended for attached job", "fn", "Attach", "job.id", req.Job.Job.ID)
15111525

15121526
return io.EOF
15131527
}

host/logmux/logmux.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ type Mux struct {
4444
// following a job, in order to wait on the appropriate group from jobWaits,
4545
// as a WaitGroup can't be waited until the counter is >0
4646
jobStarts map[string]chan struct{}
47+
// jobDones tracks jobs whose log streams have all closed, so that
48+
// late subscribers can detect that the job is already done
49+
jobDones map[string]struct{}
4750

4851
subscribersMtx sync.RWMutex
4952
subscribers map[string]map[chan message]struct{}
@@ -61,6 +64,7 @@ func New(hostID, logDir string, logger log15.Logger) *Mux {
6164
logger: logger,
6265
jobWaits: make(map[string]*sync.WaitGroup),
6366
jobStarts: make(map[string]chan struct{}),
67+
jobDones: make(map[string]struct{}),
6468
subscribers: make(map[string]map[chan message]struct{}),
6569
appLogs: make(map[string]*appLog),
6670
}
@@ -319,6 +323,7 @@ func (m *Mux) Follow(r io.ReadCloser, buffer string, msgID logagg.MsgID, config
319323
m.jobsMtx.Lock()
320324
defer m.jobsMtx.Unlock()
321325
delete(m.jobWaits, config.JobID)
326+
m.jobDones[config.JobID] = struct{}{}
322327
}()
323328
}
324329

@@ -403,14 +408,18 @@ func (m *Mux) StreamLog(appID, jobID string, history, follow bool, ch chan<- *rf
403408
}
404409

405410
// jobDoneCh returns a channel that is closed when all of the streams we are
406-
// following from the job have been closed. It will never unblock if the job has
407-
// already finished.
411+
// following from the job have been closed.
408412
func (m *Mux) jobDoneCh(jobID string, stop <-chan struct{}) <-chan struct{} {
409413
ch := make(chan struct{})
410414
go func() {
411415
defer close(ch)
412416
var started chan struct{}
413417
m.jobsMtx.Lock()
418+
// check if the job has already completed
419+
if _, done := m.jobDones[jobID]; done {
420+
m.jobsMtx.Unlock()
421+
return
422+
}
414423
// check if there is already a WaitGroup
415424
wg, ok := m.jobWaits[jobID]
416425
if !ok {
@@ -449,6 +458,7 @@ func (m *Mux) jobDoneCh(jobID string, stop <-chan struct{}) <-chan struct{} {
449458
}
450459

451460
func (m *Mux) followLog(appID, jobID string, ch chan<- *rfc5424.Message) (stream.Stream, error) {
461+
m.logger.Info("followLog starting", "app.id", appID, "job.id", jobID)
452462
s := stream.New()
453463
var jobDone <-chan struct{}
454464
if jobID != "" {

0 commit comments

Comments
 (0)