refine log handlers

app/log/internal/log_writer.go

@@ -1,85 +0,0 @@
-package internal
-
-import (
-	"context"
-	"log"
-	"os"
-
-	"v2ray.com/core/common/platform"
-)
-
-type LogEntry interface {
-	String() string
-}
-
-type LogWriter interface {
-	Log(LogEntry)
-	Close()
-}
-
-type StdOutLogWriter struct {
-	logger *log.Logger
-}
-
-func NewStdOutLogWriter() LogWriter {
-	return &StdOutLogWriter{
-		logger: log.New(os.Stdout, "", log.Ldate|log.Ltime),
-	}
-}
-
-func (w *StdOutLogWriter) Log(log LogEntry) {
-	w.logger.Print(log.String() + platform.LineSeparator())
-}
-
-func (*StdOutLogWriter) Close() {}
-
-type FileLogWriter struct {
-	queue  chan string
-	logger *log.Logger
-	file   *os.File
-	ctx    context.Context
-	cancel context.CancelFunc
-}
-
-func (w *FileLogWriter) Log(log LogEntry) {
-	select {
-	case <-w.ctx.Done():
-		return
-	case w.queue <- log.String():
-	default:
-		// We don't expect this to happen, but don't want to block main thread as well.
-	}
-}
-
-func (w *FileLogWriter) run(ctx context.Context) {
-	for {
-		select {
-		case <-ctx.Done():
-			w.file.Close()
-			return
-		case entry := <-w.queue:
-			w.logger.Print(entry + platform.LineSeparator())
-		}
-	}
-}
-
-func (w *FileLogWriter) Close() {
-	w.cancel()
-}
-
-func NewFileLogWriter(path string) (*FileLogWriter, error) {
-	file, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600)
-	if err != nil {
-		return nil, err
-	}
-	ctx, cancel := context.WithCancel(context.Background())
-	logger := &FileLogWriter{
-		queue:  make(chan string, 16),
-		logger: log.New(file, "", log.Ldate|log.Ltime),
-		file:   file,
-		ctx:    ctx,
-		cancel: cancel,
-	}
-	go logger.run(ctx)
-	return logger, nil
-}

app/log/internal/log_writer_test.go

@@ -1 +0,0 @@
-package internal_test

app/log/log.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"sync"
 
-	"v2ray.com/core/app/log/internal"
 	"v2ray.com/core/common"
 	"v2ray.com/core/common/log"
 )
@@ -15,15 +14,27 @@ import (
 type Instance struct {
 	sync.RWMutex
 	config       *Config
-	accessLogger internal.LogWriter
+	accessLogger log.Handler
-	errorLogger  internal.LogWriter
+	errorLogger  log.Handler
+	active       bool
 }
 
 // New creates a new log.Instance based on the given config.
 func New(ctx context.Context, config *Config) (*Instance, error) {
-	return &Instance{
+	g := &Instance{
 		config: config,
-	}, nil
+		active: true,
+	}
+
+	if err := g.initAccessLogger(); err != nil {
+		return nil, newError("failed to initialize access logger").Base(err).AtWarning()
+	}
+	if err := g.initErrorLogger(); err != nil {
+		return nil, newError("failed to initialize error logger").Base(err).AtWarning()
+	}
+	log.RegisterHandler(g)
+
+	return g, nil
 }
 
 // Interface implements app.Application.Interface().
@@ -34,13 +45,13 @@ func (*Instance) Interface() interface{} {
 func (g *Instance) initAccessLogger() error {
 	switch g.config.AccessLogType {
 	case LogType_File:
-		logger, err := internal.NewFileLogWriter(g.config.AccessLogPath)
+		creator, err := log.CreateFileLogWriter(g.config.AccessLogPath)
 		if err != nil {
 			return err
 		}
-		g.accessLogger = logger
+		g.accessLogger = log.NewLogger(creator)
 	case LogType_Console:
-		g.accessLogger = internal.NewStdOutLogWriter()
+		g.accessLogger = log.NewLogger(log.CreateStdoutLogWriter())
 	default:
 	}
 	return nil
@@ -49,13 +60,13 @@ func (g *Instance) initAccessLogger() error {
 func (g *Instance) initErrorLogger() error {
 	switch g.config.ErrorLogType {
 	case LogType_File:
-		logger, err := internal.NewFileLogWriter(g.config.ErrorLogPath)
+		creator, err := log.CreateFileLogWriter(g.config.ErrorLogPath)
 		if err != nil {
 			return err
 		}
-		g.errorLogger = logger
+		g.errorLogger = log.NewLogger(creator)
 	case LogType_Console:
-		g.errorLogger = internal.NewStdOutLogWriter()
+		g.errorLogger = log.NewLogger(log.CreateStdoutLogWriter())
 	default:
 	}
 	return nil
@@ -63,32 +74,33 @@ func (g *Instance) initErrorLogger() error {
 
 // Start implements app.Application.Start().
 func (g *Instance) Start() error {
-	if err := g.initAccessLogger(); err != nil {
+	g.Lock()
-		return newError("failed to initialize access logger").Base(err).AtWarning()
+	defer g.Unlock()
-	}
+	g.active = true
-	if err := g.initErrorLogger(); err != nil {
-		return newError("failed to initialize error logger").Base(err).AtWarning()
-	}
-	log.RegisterHandler(g)
 	return nil
 }
 
+func (g *Instance) isActive() bool {
+	g.RLock()
+	defer g.RUnlock()
+
+	return g.active
+}
+
 // Handle implements log.Handler.
 func (g *Instance) Handle(msg log.Message) {
+	if !g.isActive() {
+		return
+	}
+
 	switch msg := msg.(type) {
 	case *log.AccessMessage:
-		g.RLock()
-		defer g.RUnlock()
 		if g.accessLogger != nil {
-			g.accessLogger.Log(msg)
+			g.accessLogger.Handle(msg)
 		}
 	case *log.GeneralMessage:
-		if msg.Severity.SevererThan(g.config.ErrorLogLevel) {
+		if g.errorLogger != nil && msg.Severity <= g.config.ErrorLogLevel {
-			g.RLock()
+			g.errorLogger.Handle(msg)
-			defer g.RUnlock()
-			if g.errorLogger != nil {
-				g.errorLogger.Log(msg)
-			}
 		}
 	default:
 		// Swallow
@@ -100,15 +112,7 @@ func (g *Instance) Close() {
 	g.Lock()
 	defer g.Unlock()
 
-	if g.accessLogger != nil {
+	g.active = true
-		g.accessLogger.Close()
-		g.accessLogger = nil
-	}
-
-	if g.errorLogger != nil {
-		g.errorLogger.Close()
-		g.errorLogger = nil
-	}
 }
 
 func init() {

common/log/log.go

@@ -17,10 +17,6 @@ type Handler interface {
 	Handle(msg Message)
 }
 
-type noOpHandler byte
-
-func (noOpHandler) Handle(msg Message) {}
-
 // GeneralMessage is a general log message that can contain all kind of content.
 type GeneralMessage struct {
 	Severity Severity
@@ -32,10 +28,6 @@ func (m *GeneralMessage) String() string {
 	return serial.Concat("[", m.Severity, "]: ", m.Content)
 }
 
-func (s Severity) SevererThan(another Severity) bool {
-	return s <= another
-}
-
 // Record writes a message into log stream.
 func Record(msg Message) {
 	h := (*Handler)(atomic.LoadPointer(&logHandler))
@@ -53,7 +45,3 @@ func RegisterHandler(handler Handler) {
 	}
 	atomic.StorePointer(&logHandler, unsafe.Pointer(&handler))
 }
-
-func init() {
-	RegisterHandler(noOpHandler(0))
-}

common/log/logger.go

@@ -0,0 +1,132 @@
+package log
+
+import (
+	"io"
+	"log"
+	"os"
+	"time"
+
+	"v2ray.com/core/common/platform"
+	"v2ray.com/core/common/signal"
+)
+
+type LogWriter interface {
+	io.WriteCloser
+}
+
+type LogWriterCreator func() LogWriter
+
+type generalLogger struct {
+	creator LogWriterCreator
+	buffer  chan Message
+	access  *signal.Semaphore
+}
+
+// NewLogger returns a generic log handler that can handle all type of messages.
+func NewLogger(logWriterCreator LogWriterCreator) Handler {
+	return &generalLogger{
+		creator: logWriterCreator,
+		buffer:  make(chan Message, 16),
+		access:  signal.NewSemaphore(1),
+	}
+}
+
+func (l *generalLogger) run() {
+	defer l.access.Signal()
+
+	dataWritten := false
+	ticker := time.NewTicker(time.Minute)
+	defer ticker.Stop()
+
+	logger := l.creator()
+	if logger == nil {
+		return
+	}
+	defer logger.Close()
+
+	ls := []byte(platform.LineSeparator())
+
+	for {
+		select {
+		case msg := <-l.buffer:
+			logger.Write([]byte(msg.String()))
+			logger.Write(ls)
+			dataWritten = true
+		case <-ticker.C:
+			if !dataWritten {
+				return
+			}
+			dataWritten = false
+		}
+	}
+}
+
+func (l *generalLogger) Handle(msg Message) {
+	select {
+	case l.buffer <- msg:
+	default:
+	}
+
+	select {
+	case <-l.access.Wait():
+		go l.run()
+	default:
+	}
+}
+
+type consoleLogWriter struct {
+	logger *log.Logger
+}
+
+func (w *consoleLogWriter) Write(b []byte) (int, error) {
+	w.logger.Print(string(b))
+	return len(b), nil
+}
+
+func (w *consoleLogWriter) Close() error {
+	return nil
+}
+
+type fileLogWriter struct {
+	file   *os.File
+	logger *log.Logger
+}
+
+func (w *fileLogWriter) Write(b []byte) (int, error) {
+	w.logger.Print(string(b))
+	return len(b), nil
+}
+
+func (w *fileLogWriter) Close() error {
+	return w.file.Close()
+}
+
+func CreateStdoutLogWriter() LogWriterCreator {
+	return func() LogWriter {
+		return &consoleLogWriter{
+			logger: log.New(os.Stdout, "", log.Ldate|log.Ltime),
+		}
+	}
+}
+
+func CreateFileLogWriter(path string) (LogWriterCreator, error) {
+	file, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600)
+	if err != nil {
+		return nil, err
+	}
+	file.Close()
+	return func() LogWriter {
+		file, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600)
+		if err != nil {
+			return nil
+		}
+		return &fileLogWriter{
+			file:   file,
+			logger: log.New(file, "", log.Ldate|log.Ltime),
+		}
+	}, nil
+}
+
+func init() {
+	RegisterHandler(NewLogger(CreateStdoutLogWriter()))
+}