Skip to content

Commit

Permalink
sdk: avoid using different state (open-policy-agent#4505)
Browse files Browse the repository at this point in the history
I noticed that when operating on opa.state, locking is usually done to avoid
a race, whereas here opa.state is used directly. By comparing the previous
changes, I found that open-policy-agent#4240 changed the previous behaviour.

This change adjusts that: we ensure that we work on the state as read via
the mutex-protected `s := *opa.state`.

Signed-off-by: Iceber Gu <wei.cai-nat@daocloud.io>
  • Loading branch information
Iceber authored and rokkiter committed Apr 18, 2022
1 parent ea94481 commit 4f8a463
Showing 1 changed file with 14 additions and 14 deletions.
28 changes: 14 additions & 14 deletions sdk/opa.go
Expand Up @@ -224,15 +224,15 @@ func (opa *OPA) Decision(ctx context.Context, options DecisionOptions) (*Decisio
result, err := opa.executeTransaction(
ctx,
&record,
func(result *DecisionResult) {
func(s state, result *DecisionResult) {
result.Result, record.InputAST, record.Bundles, record.Error = evaluate(ctx, evalArgs{
runtime: opa.state.manager.Info,
printHook: opa.state.manager.PrintHook(),
compiler: opa.state.manager.GetCompiler(),
store: opa.state.manager.Store,
runtime: s.manager.Info,
printHook: s.manager.PrintHook(),
compiler: s.manager.GetCompiler(),
store: s.manager.Store,
queryCache: s.queryCache,
interQueryCache: s.interQueryBuiltinCache,
txn: record.Txn,
queryCache: opa.state.queryCache,
interQueryCache: opa.state.interQueryBuiltinCache,
now: record.Timestamp,
path: record.Path,
input: *record.Input,
Expand Down Expand Up @@ -272,7 +272,7 @@ func newDecisionResult() (*DecisionResult, error) {
return result, nil
}

func (opa *OPA) executeTransaction(ctx context.Context, record *server.Info, work func(result *DecisionResult)) (*DecisionResult, error) {
func (opa *OPA) executeTransaction(ctx context.Context, record *server.Info, work func(state, *DecisionResult)) (*DecisionResult, error) {
m := metrics.New()
m.Timer(metrics.SDKDecisionEval).Start()

Expand Down Expand Up @@ -300,7 +300,7 @@ func (opa *OPA) executeTransaction(ctx context.Context, record *server.Info, wor

if record.Error == nil {
defer s.manager.Store.Abort(ctx, record.Txn)
work(result)
work(s, result)
}

m.Timer(metrics.SDKDecisionEval).Stop()
Expand Down Expand Up @@ -330,12 +330,12 @@ func (opa *OPA) Partial(ctx context.Context, options PartialOptions) (*PartialRe
decision, err := opa.executeTransaction(
ctx,
&record,
func(result *DecisionResult) {
func(s state, result *DecisionResult) {
pq, record.InputAST, record.Bundles, record.Error = partial(ctx, partialEvalArgs{
runtime: opa.state.manager.Info,
printHook: opa.state.manager.PrintHook(),
compiler: opa.state.manager.GetCompiler(),
store: opa.state.manager.Store,
runtime: s.manager.Info,
printHook: s.manager.PrintHook(),
compiler: s.manager.GetCompiler(),
store: s.manager.Store,
txn: record.Txn,
now: record.Timestamp,
query: record.Query,
Expand Down

0 comments on commit 4f8a463

Please sign in to comment.