Skip to content

Commit

Permalink
make InitFlags read-only
Browse files Browse the repository at this point in the history
The way it was implemented before, the current value of several flags was
written to the global config struct each time InitFlags was called. Not all
flags may get written concurrently, so this led to a data race when some other
goroutine was actively using klog. InitFlags also created new flag.Flag
instances for each call.

By creating flags once during init, InitFlags becomes entirely read-only and
thus safe to use concurrently.

What remains ambiguous is which flags may get changed at runtime. That was
underspecified before and doesn't get fixed here either, it just gets called
out as a potential problem.
  • Loading branch information
pohly committed Sep 7, 2022
1 parent a952486 commit fe3157c
Showing 1 changed file with 32 additions and 32 deletions.
64 changes: 32 additions & 32 deletions klog.go
Expand Up @@ -396,45 +396,51 @@ type flushSyncWriter interface {
io.Writer
}

// init sets up the defaults.
var logging = loggingT{
settings: settings{
contextualLoggingEnabled: true,
},
}
var commandLine flag.FlagSet

// init sets up the defaults and creates command line flags.
func init() {
commandLine.StringVar(&logging.logDir, "log_dir", "", "If non-empty, write log files in this directory (no effect when -logtostderr=true)")
commandLine.StringVar(&logging.logFile, "log_file", "", "If non-empty, use this log file (no effect when -logtostderr=true)")
commandLine.Uint64Var(&logging.logFileMaxSizeMB, "log_file_max_size", 1800,
"Defines the maximum size a log file can grow to (no effect when -logtostderr=true). Unit is megabytes. "+
"If the value is 0, the maximum file size is unlimited.")
commandLine.BoolVar(&logging.toStderr, "logtostderr", true, "log to standard error instead of files")
commandLine.BoolVar(&logging.alsoToStderr, "alsologtostderr", false, "log to standard error as well as files (no effect when -logtostderr=true)")
logging.setVState(0, nil, false)
commandLine.Var(&logging.verbosity, "v", "number for the log level verbosity")
commandLine.BoolVar(&logging.addDirHeader, "add_dir_header", false, "If true, adds the file directory to the header of the log messages")
commandLine.BoolVar(&logging.skipHeaders, "skip_headers", false, "If true, avoid header prefixes in the log messages")
commandLine.BoolVar(&logging.oneOutput, "one_output", false, "If true, only write logs to their native severity level (vs also writing to each lower severity level; no effect when -logtostderr=true)")
commandLine.BoolVar(&logging.skipLogHeaders, "skip_log_headers", false, "If true, avoid headers when opening log files (no effect when -logtostderr=true)")
logging.stderrThreshold = severityValue{
Severity: severity.ErrorLog, // Default stderrThreshold is ERROR.
}
logging.setVState(0, nil, false)
logging.logDir = ""
logging.logFile = ""
logging.logFileMaxSizeMB = 1800
logging.toStderr = true
logging.alsoToStderr = false
logging.skipHeaders = false
logging.addDirHeader = false
logging.skipLogHeaders = false
logging.oneOutput = false
commandLine.Var(&logging.stderrThreshold, "stderrthreshold", "logs at or above this threshold go to stderr when writing to files and stderr (no effect when -logtostderr=true or -alsologtostderr=false)")
commandLine.Var(&logging.vmodule, "vmodule", "comma-separated list of pattern=N settings for file-filtered logging")
commandLine.Var(&logging.traceLocation, "log_backtrace_at", "when logging hits line file:N, emit a stack trace")

logging.flushD = newFlushDaemon(logging.lockAndFlushAll, nil)
}

// InitFlags is for explicitly initializing the flags.
// It may get called repeatedly for different flagsets, but not
// twice for the same one. May get called concurrently
// to other goroutines using klog. However, only some flags
// may get set concurrently (see implementation).
func InitFlags(flagset *flag.FlagSet) {
if flagset == nil {
flagset = flag.CommandLine
}

flagset.StringVar(&logging.logDir, "log_dir", logging.logDir, "If non-empty, write log files in this directory (no effect when -logtostderr=true)")
flagset.StringVar(&logging.logFile, "log_file", logging.logFile, "If non-empty, use this log file (no effect when -logtostderr=true)")
flagset.Uint64Var(&logging.logFileMaxSizeMB, "log_file_max_size", logging.logFileMaxSizeMB,
"Defines the maximum size a log file can grow to (no effect when -logtostderr=true). Unit is megabytes. "+
"If the value is 0, the maximum file size is unlimited.")
flagset.BoolVar(&logging.toStderr, "logtostderr", logging.toStderr, "log to standard error instead of files")
flagset.BoolVar(&logging.alsoToStderr, "alsologtostderr", logging.alsoToStderr, "log to standard error as well as files (no effect when -logtostderr=true)")
flagset.Var(&logging.verbosity, "v", "number for the log level verbosity")
flagset.BoolVar(&logging.addDirHeader, "add_dir_header", logging.addDirHeader, "If true, adds the file directory to the header of the log messages")
flagset.BoolVar(&logging.skipHeaders, "skip_headers", logging.skipHeaders, "If true, avoid header prefixes in the log messages")
flagset.BoolVar(&logging.oneOutput, "one_output", logging.oneOutput, "If true, only write logs to their native severity level (vs also writing to each lower severity level; no effect when -logtostderr=true)")
flagset.BoolVar(&logging.skipLogHeaders, "skip_log_headers", logging.skipLogHeaders, "If true, avoid headers when opening log files (no effect when -logtostderr=true)")
flagset.Var(&logging.stderrThreshold, "stderrthreshold", "logs at or above this threshold go to stderr when writing to files and stderr (no effect when -logtostderr=true or -alsologtostderr=false)")
flagset.Var(&logging.vmodule, "vmodule", "comma-separated list of pattern=N settings for file-filtered logging")
flagset.Var(&logging.traceLocation, "log_backtrace_at", "when logging hits line file:N, emit a stack trace")
commandLine.VisitAll(func(f *flag.Flag) {
flagset.Var(f.Value, f.Name, f.Usage)
})
}

// Flush flushes all pending log I/O.
Expand Down Expand Up @@ -549,12 +555,6 @@ type loggingT struct {
vmap map[uintptr]Level
}

var logging = loggingT{
settings: settings{
contextualLoggingEnabled: true,
},
}

// setVState sets a consistent state for V logging.
// l.mu is held.
func (l *loggingT) setVState(verbosity Level, filter []modulePat, setFilter bool) {
Expand Down

0 comments on commit fe3157c

Please sign in to comment.