Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deadlock in version 1.9.3 and a full test for reproducing the issue #1448

Open
sillydong opened this issue Nov 28, 2024 · 2 comments
Open

Deadlock in version 1.9.3 and a full test for reproducing the issue #1448

sillydong opened this issue Nov 28, 2024 · 2 comments
Labels

Comments

@sillydong
Copy link

sillydong commented Nov 28, 2024

This is related to #1440

I got a simple test function to reproduce the scenario.

package playground_test

import (
	"fmt"
	"testing"
	"time"

	"github.com/sirupsen/logrus"
)

type XXX struct {
	X string `json:"x"`
}

func (x XXX) MarshalJSON() ([]byte, error) {
	fmt.Println("aaa")
	logrus.Info("second")
	return []byte(`{"x":"y"}`), nil
}

func TestLog(t *testing.T) {
	jsonFormatter := &logrus.JSONFormatter{TimestampFormat: time.RFC3339Nano}
	logrus.StandardLogger().SetFormatter(jsonFormatter)

	x := XXX{}

	logrus.WithFields(logrus.Fields{
		"a": "b",
		"c": x,
	}).Info("first")
}

This case is like when we want to log something, the fields contains a value which has its own function for marshaling, and in that marshal function, it logged again. It's like log in log.

From the test output, we can see aaa printed, and second will not be printed.
image
If we comment out line logging second, we can see both aaa and first in output
image

Internally, it is caused in the second time of calling log and it trying to acquire Logger.mu in Entry.log.
image
The lock is held by the first call, so here it will blocked.

Further more, it is caused by the change in write function
image
lock is moved from after format to before format. So when it's running the format function, logger is locked and that time when people want to log another content, it cannot get lock and do logging.

For a log lib, people may want to log anywhere they want. So such case is really a possible daily usage for people. We cannot stop people from call log function inside a log call. We cannot review every structure in log field to ensure it's marshaling function has no log call. So I think this worth be fixed.

@sillydong
Copy link
Author

relate to this PR #1263

Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant