Skip to content

Commit 35240da

Browse files
committed
fix
1 parent 7de46b0 commit 35240da

File tree

5 files changed

+36
-27
lines changed

5 files changed

+36
-27
lines changed

modules/log/event_writer_base.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func eventWriterStartGo(ctx context.Context, w EventWriter, shared bool) {
149149
if shared {
150150
ctxDesc = "Logger: EventWriter (shared): " + w.GetWriterName()
151151
}
152-
writerCtx, writerCancel := newContext(ctx, ctxDesc)
152+
writerCtx, writerCancel := newProcessTypedContext(ctx, ctxDesc)
153153
go func() {
154154
defer writerCancel()
155155
defer close(w.Base().stopped)

modules/log/init.go

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,12 @@ import (
77
"context"
88
"runtime"
99
"strings"
10-
"sync/atomic"
1110

1211
"code.gitea.io/gitea/modules/process"
1312
"code.gitea.io/gitea/modules/util/rotatingfilewriter"
1413
)
1514

16-
var (
17-
projectPackagePrefix string
18-
processTraceDisabled atomic.Int64
19-
)
15+
var projectPackagePrefix string
2016

2117
func init() {
2218
_, filename, _, _ := runtime.Caller(0)
@@ -28,25 +24,21 @@ func init() {
2824

2925
rotatingfilewriter.ErrorPrintf = FallbackErrorf
3026

31-
process.Trace = func(start bool, pid process.IDType, description string, parentPID process.IDType, typ string) {
32-
// the logger manager has its own mutex lock, so it's safe to use "Load" here
33-
if processTraceDisabled.Load() != 0 {
34-
return
35-
}
27+
process.TraceCallback = func(skip int, start bool, pid process.IDType, description string, parentPID process.IDType, typ string) {
3628
if start && parentPID != "" {
37-
Log(1, TRACE, "Start %s: %s (from %s) (%s)", NewColoredValue(pid, FgHiYellow), description, NewColoredValue(parentPID, FgYellow), NewColoredValue(typ, Reset))
29+
Log(skip+1, TRACE, "Start %s: %s (from %s) (%s)", NewColoredValue(pid, FgHiYellow), description, NewColoredValue(parentPID, FgYellow), NewColoredValue(typ, Reset))
3830
} else if start {
39-
Log(1, TRACE, "Start %s: %s (%s)", NewColoredValue(pid, FgHiYellow), description, NewColoredValue(typ, Reset))
31+
Log(skip+1, TRACE, "Start %s: %s (%s)", NewColoredValue(pid, FgHiYellow), description, NewColoredValue(typ, Reset))
4032
} else {
41-
Log(1, TRACE, "Done %s: %s", NewColoredValue(pid, FgHiYellow), NewColoredValue(description, Reset))
33+
Log(skip+1, TRACE, "Done %s: %s", NewColoredValue(pid, FgHiYellow), NewColoredValue(description, Reset))
4234
}
4335
}
4436
}
4537

46-
func newContext(parent context.Context, desc string) (ctx context.Context, cancel context.CancelFunc) {
38+
func newProcessTypedContext(parent context.Context, desc string) (ctx context.Context, cancel context.CancelFunc) {
4739
// the "process manager" also calls "log.Trace()" to output logs, so if we want to create new contexts by the manager, we need to disable the trace temporarily
48-
processTraceDisabled.Add(1)
49-
defer processTraceDisabled.Add(-1)
40+
process.TraceLogDisable(true)
41+
defer process.TraceLogDisable(false)
5042
ctx, _, cancel = process.GetManager().AddTypedContext(parent, desc, process.SystemProcessType, false)
5143
return ctx, cancel
5244
}

modules/log/logger_impl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func (l *LoggerImpl) GetLevel() Level {
230230

231231
func NewLoggerWithWriters(ctx context.Context, name string, writer ...EventWriter) *LoggerImpl {
232232
l := &LoggerImpl{}
233-
l.ctx, l.ctxCancel = newContext(ctx, "Logger: "+name)
233+
l.ctx, l.ctxCancel = newProcessTypedContext(ctx, "Logger: "+name)
234234
l.LevelLogger = BaseLoggerToGeneralLogger(l)
235235
l.eventWriters = map[string]EventWriter{}
236236
l.syncLevelInternal()

modules/log/manager.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,6 @@ func GetManager() *LoggerManager {
137137

138138
func NewManager() *LoggerManager {
139139
m := &LoggerManager{writers: map[string]EventWriter{}, loggers: map[string]*LoggerImpl{}}
140-
m.ctx, m.ctxCancel = newContext(context.Background(), "LoggerManager")
140+
m.ctx, m.ctxCancel = newProcessTypedContext(context.Background(), "LoggerManager")
141141
return m
142142
}

modules/process/manager.go

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ package process
66

77
import (
88
"context"
9-
"log"
109
"runtime/pprof"
1110
"strconv"
1211
"sync"
12+
"sync/atomic"
1313
"time"
1414
)
1515

@@ -44,16 +44,33 @@ type IDType string
4444
// - it is simply an alias for context.CancelFunc and is only for documentary purposes
4545
type FinishedFunc = context.CancelFunc
4646

47-
var Trace = defaultTrace // this global can be overridden by particular logging packages - thus avoiding import cycles
47+
var (
48+
traceDisabled atomic.Int64
49+
TraceCallback = defaultTraceCallback // this global can be overridden by particular logging packages - thus avoiding import cycles
50+
)
51+
52+
// defaultTraceCallback is a no-op. Without a proper TraceCallback (provided by the logger system), this "Trace" level messages shouldn't be outputted.
53+
func defaultTraceCallback(skip int, start bool, pid IDType, description string, parentPID IDType, typ string) {
54+
}
4855

49-
func defaultTrace(start bool, pid IDType, description string, parentPID IDType, typ string) {
50-
if start && parentPID != "" {
51-
log.Printf("start process %s: %s (from %s) (%s)", pid, description, parentPID, typ)
52-
} else if start {
53-
log.Printf("start process %s: %s (%s)", pid, description, typ)
56+
// TraceLogDisable disables (or revert the disabling) the trace log for the process lifecycle.
57+
// eg: the logger system shouldn't print the trace log for themselves, that's cycle dependency (Logger -> ProcessManager -> TraceCallback -> Logger ...)
58+
// Theoretically, such trace log should only be enabled when the logger system is ready with a proper level, so the default TraceCallback is a no-op.
59+
func TraceLogDisable(v bool) {
60+
if v {
61+
traceDisabled.Add(1)
5462
} else {
55-
log.Printf("end process %s: %s", pid, description)
63+
traceDisabled.Add(-1)
64+
}
65+
}
66+
67+
func Trace(start bool, pid IDType, description string, parentPID IDType, typ string) {
68+
if traceDisabled.Load() != 0 {
69+
// the traceDisabled counter is mainly for recursive calls, so no concurrency problem.
70+
// because the counter can't be 0 since the caller function hasn't returned (decreased the counter) yet.
71+
return
5672
}
73+
TraceCallback(1, start, pid, description, parentPID, typ)
5774
}
5875

5976
// Manager manages all processes and counts PIDs.

0 commit comments

Comments
 (0)