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

fatal error: concurrent map writes #1666

Closed
liorlevtov opened this issue Aug 25, 2019 · 1 comment · Fixed by #1667
Closed

fatal error: concurrent map writes #1666

liorlevtov opened this issue Aug 25, 2019 · 1 comment · Fixed by #1667
Labels

Comments

@liorlevtov
Copy link

Expected Behavior

Opa should not panic and exist

Actual Behavior

panics

Steps to Reproduce the Problem

  1. during everyday operation we saw panic in the log

Additional Info

attached the whole stack trace that OPA
OPA_Full_trace.txt

@tsandall tsandall added the bug label Aug 25, 2019
@tsandall
Copy link
Member

tsandall commented Aug 25, 2019

I think this is caused by this change: 66fb55f#diff-304927c4591ee459ab6e909666e945d0R662. The server goroutines share an instance of the compiler. On this line the compiler's map could be modified concurrently. I'll review the calling code and figure out the right solution.

EDIT:

Here is a small test case for the server that reproduces the issue:


func TestConcurrentQuery(t *testing.T) {

	fixture := newFixture(t)

	var wg sync.WaitGroup

	for i := 0; i < 4; i++ {
		wg.Add(1)
		go func() {
			for x := 0; x < 10000; x++ {
				if err := fixture.v1("POST", "/data", "", 200, ``); err != nil {
					t.Fatal(err)
				}
			}
			wg.Done()
		}()
	}

	wg.Wait()
}

tsandall added a commit to tsandall/opa that referenced this issue Aug 25, 2019
Previously the rego package was modifying the compiler's unsafe
built-in set each time a query was evaluated or prepared. This
resulted in concurrent write panics in the server (since the server
and other callers assume the compiler is immutable and can be shared
across goroutines.)

These changes update how unsafe built-ins are specified. The query
compiler continues to inherit the set from the compiler but there are
two important differences:

1. Callers can provide unsafe built-ins to the query compiler. This
allows callers to override the behaviour of the compiler if they need
to.

2. The rego package does not set unsafe built-ins if the compiler is
provided by the caller. This ensures that the compiler is not modified
concurrently.

With these changes the rego package doesn't union unsafe built-ins
like it used to. Since this feature was only added recently it's
unlikely anyone is relying on it.

Fixes open-policy-agent#1666

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
tsandall added a commit that referenced this issue Aug 26, 2019
Previously the rego package was modifying the compiler's unsafe
built-in set each time a query was evaluated or prepared. This
resulted in concurrent write panics in the server (since the server
and other callers assume the compiler is immutable and can be shared
across goroutines.)

These changes update how unsafe built-ins are specified. The query
compiler continues to inherit the set from the compiler but there are
two important differences:

1. Callers can provide unsafe built-ins to the query compiler. This
allows callers to override the behaviour of the compiler if they need
to.

2. The rego package does not set unsafe built-ins if the compiler is
provided by the caller. This ensures that the compiler is not modified
concurrently.

With these changes the rego package doesn't union unsafe built-ins
like it used to. Since this feature was only added recently it's
unlikely anyone is relying on it.

Fixes #1666

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
tsandall added a commit to tsandall/opa that referenced this issue Aug 26, 2019
Previously the rego package was modifying the compiler's unsafe
built-in set each time a query was evaluated or prepared. This
resulted in concurrent write panics in the server (since the server
and other callers assume the compiler is immutable and can be shared
across goroutines.)

These changes update how unsafe built-ins are specified. The query
compiler continues to inherit the set from the compiler but there are
two important differences:

1. Callers can provide unsafe built-ins to the query compiler. This
allows callers to override the behaviour of the compiler if they need
to.

2. The rego package does not set unsafe built-ins if the compiler is
provided by the caller. This ensures that the compiler is not modified
concurrently.

With these changes the rego package doesn't union unsafe built-ins
like it used to. Since this feature was only added recently it's
unlikely anyone is relying on it.

Fixes open-policy-agent#1666

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants