From e7f7cbfad2fc850e1ea0306a8083e8f3b5f70982 Mon Sep 17 00:00:00 2001 From: Yu Liu Date: Tue, 13 Jun 2023 18:50:03 +0000 Subject: [PATCH] Fix a race problem in sandboxing genrule logic. Bug: 286978650 Test: locally build Change-Id: I4fab03f74f5634666d22f7ed6e18ee69f780d6ca --- genrule/genrule.go | 53 +++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/genrule/genrule.go b/genrule/genrule.go index 4992625e5..50262d2e4 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -24,7 +24,6 @@ import ( "path/filepath" "strconv" "strings" - "sync" "android/soong/bazel/cquery" @@ -61,12 +60,6 @@ var PrepareForIntegrationTestWithGenrule = android.GroupFixturePreparers( PrepareForTestWithGenRuleBuildComponents, ) -var DepfileAllowSet map[string]bool -var SandboxingDenyModuleSet map[string]bool -var SandboxingDenyPathSet map[string]bool -var SandboxingDenyModuleSetLock sync.Mutex -var DepfileAllowSetLock sync.Mutex - func RegisterGenruleBuildComponents(ctx android.RegistrationContext) { ctx.RegisterModuleType("genrule_defaults", defaultsFactory) @@ -602,15 +595,10 @@ func (g *Module) GenerateAndroidBuildActions(ctx android.ModuleContext) { // Allowlist genrule to use depfile until we have a solution to remove it. // TODO(b/235582219): Remove allowlist for genrule if Bool(g.properties.Depfile) { - if DepfileAllowSet == nil { - DepfileAllowSetLock.Lock() - defer DepfileAllowSetLock.Unlock() - DepfileAllowSet = map[string]bool{} - android.AddToStringSet(DepfileAllowSet, DepfileAllowList) - } + sandboxingAllowlistSets := getSandboxingAllowlistSets(ctx) // TODO(b/283852474): Checking the GenruleSandboxing flag is temporary in // order to pass the presubmit before internal master is updated. - if ctx.DeviceConfig().GenruleSandboxing() && !DepfileAllowSet[g.Name()] { + if ctx.DeviceConfig().GenruleSandboxing() && !sandboxingAllowlistSets.depfileAllowSet[g.Name()] { ctx.PropertyErrorf( "depfile", "Deprecated to ensure the module type is convertible to Bazel. "+ @@ -1036,20 +1024,37 @@ func DefaultsFactory(props ...interface{}) android.Module { return module } +var sandboxingAllowlistKey = android.NewOnceKey("genruleSandboxingAllowlistKey") + +type sandboxingAllowlistSets struct { + sandboxingDenyModuleSet map[string]bool + sandboxingDenyPathSet map[string]bool + depfileAllowSet map[string]bool +} + +func getSandboxingAllowlistSets(ctx android.PathContext) *sandboxingAllowlistSets { + return ctx.Config().Once(sandboxingAllowlistKey, func() interface{} { + sandboxingDenyModuleSet := map[string]bool{} + sandboxingDenyPathSet := map[string]bool{} + depfileAllowSet := map[string]bool{} + + android.AddToStringSet(sandboxingDenyModuleSet, append(DepfileAllowList, SandboxingDenyModuleList...)) + android.AddToStringSet(sandboxingDenyPathSet, SandboxingDenyPathList) + android.AddToStringSet(depfileAllowSet, DepfileAllowList) + return &sandboxingAllowlistSets{ + sandboxingDenyModuleSet: sandboxingDenyModuleSet, + sandboxingDenyPathSet: sandboxingDenyPathSet, + depfileAllowSet: depfileAllowSet, + } + }).(*sandboxingAllowlistSets) +} func getSandboxedRuleBuilder(ctx android.ModuleContext, r *android.RuleBuilder) *android.RuleBuilder { if !ctx.DeviceConfig().GenruleSandboxing() { return r.SandboxTools() } - if SandboxingDenyModuleSet == nil { - SandboxingDenyModuleSetLock.Lock() - defer SandboxingDenyModuleSetLock.Unlock() - SandboxingDenyModuleSet = map[string]bool{} - SandboxingDenyPathSet = map[string]bool{} - android.AddToStringSet(SandboxingDenyModuleSet, append(DepfileAllowList, SandboxingDenyModuleList...)) - android.AddToStringSet(SandboxingDenyPathSet, SandboxingDenyPathList) - } - - if SandboxingDenyPathSet[ctx.ModuleDir()] || SandboxingDenyModuleSet[ctx.ModuleName()] { + sandboxingAllowlistSets := getSandboxingAllowlistSets(ctx) + if sandboxingAllowlistSets.sandboxingDenyPathSet[ctx.ModuleDir()] || + sandboxingAllowlistSets.sandboxingDenyModuleSet[ctx.ModuleName()] { return r.SandboxTools() } return r.SandboxInputs()