Skip to content

Commit 6d931c9

Browse files
basel1322attiasas
andauthored
Fixed concurrent-map-issue (#710)
Co-authored-by: Assaf Attias <49212512+attiasas@users.noreply.github.com>
1 parent 70b8e6f commit 6d931c9

2 files changed

Lines changed: 62 additions & 3 deletions

File tree

commands/curation/curationaudit.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,8 @@ func (nc *treeAnalyzer) fetchNodeStatus(node xrayUtils.GraphNode, p *sync.Map) e
860860
name = scope + "/" + name
861861
}
862862
for _, packageUrl := range packageUrls {
863-
resp, _, err := nc.rtManager.Client().SendHead(packageUrl, &nc.httpClientDetails)
863+
requestDetails := nc.httpClientDetails.Clone()
864+
resp, _, err := nc.rtManager.Client().SendHead(packageUrl, requestDetails)
864865
if err != nil {
865866
if resp != nil && resp.StatusCode >= 400 {
866867
return errorutils.CheckErrorf(errorTemplateHeadRequest, packageUrl, name, version, resp.StatusCode, err)
@@ -897,8 +898,9 @@ func (nc *treeAnalyzer) fetchNodeStatus(node xrayUtils.GraphNode, p *sync.Map) e
897898

898899
// We try to collect curation details from GET response after HEAD request got forbidden status code.
899900
func (nc *treeAnalyzer) getBlockedPackageDetails(packageUrl string, name string, version string) (*PackageStatus, error) {
900-
nc.httpClientDetails.Headers["X-Artifactory-Curation-Request-Waiver"] = "syn"
901-
getResp, respBody, _, err := nc.rtManager.Client().SendGet(packageUrl, true, &nc.httpClientDetails)
901+
requestDetails := nc.httpClientDetails.Clone()
902+
requestDetails.Headers["X-Artifactory-Curation-Request-Waiver"] = "syn"
903+
getResp, respBody, _, err := nc.rtManager.Client().SendGet(packageUrl, true, requestDetails)
902904
if err != nil {
903905
if getResp == nil {
904906
return nil, err

commands/curation/curationaudit_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1636,3 +1636,60 @@ func TestSendWaiverRequests(t *testing.T) {
16361636
})
16371637
}
16381638
}
1639+
1640+
// TestFetchNodesStatusConcurrentMapWrite reproduces crash
1641+
// reported when many packages are blocked by curation simultaneously.
1642+
func TestFetchNodesStatusConcurrentMapWrite(t *testing.T) {
1643+
const numNodes = 50
1644+
1645+
// Mock server: HEAD returns 403 for all packages, GET returns curation block JSON
1646+
blockResponse := `{"errors":[{"status":403,"message":"Package download was blocked by JFrog Packages Curation service due to the following policies violated {testPolicy, testCondition, testExplanation, testRecommendation}"}]}`
1647+
serverMock, _, rtManager := coreCommonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) {
1648+
if r.Method == http.MethodHead {
1649+
w.WriteHeader(http.StatusForbidden)
1650+
return
1651+
}
1652+
if r.Method == http.MethodGet {
1653+
w.WriteHeader(http.StatusForbidden)
1654+
_, _ = w.Write([]byte(blockResponse))
1655+
return
1656+
}
1657+
})
1658+
defer serverMock.Close()
1659+
1660+
rtAuth := rtManager.GetConfig().GetServiceDetails()
1661+
httpClientDetails := rtAuth.CreateHttpClientDetails()
1662+
1663+
root := &xrayUtils.GraphNode{Id: "npm://root:1.0.0"}
1664+
for i := 0; i < numNodes; i++ {
1665+
root.Nodes = append(root.Nodes, &xrayUtils.GraphNode{
1666+
Id: fmt.Sprintf("npm://pkg-%d:%d.0.0", i, i),
1667+
})
1668+
}
1669+
1670+
analyzer := treeAnalyzer{
1671+
rtManager: rtManager,
1672+
extractPoliciesRegex: regexp.MustCompile(extractPoliciesRegexTemplate),
1673+
rtAuth: rtAuth,
1674+
httpClientDetails: httpClientDetails,
1675+
url: rtAuth.GetUrl(),
1676+
repo: "npm-remote",
1677+
tech: techutils.Npm,
1678+
parallelRequests: 10,
1679+
}
1680+
1681+
packagesStatusMap := sync.Map{}
1682+
rootNodes := map[string]struct{}{root.Id: {}}
1683+
1684+
// This will crash with "concurrent map writes" without the fix
1685+
err := analyzer.fetchNodesStatus(root, &packagesStatusMap, rootNodes)
1686+
assert.NoError(t, err)
1687+
1688+
// Verify all blocked packages were recorded
1689+
count := 0
1690+
packagesStatusMap.Range(func(_, _ any) bool {
1691+
count++
1692+
return true
1693+
})
1694+
assert.Equal(t, numNodes, count, "expected all %d packages to be recorded as blocked", numNodes)
1695+
}

0 commit comments

Comments
 (0)