Race Condition in BuildV3Model

[califlower]

Hi, we are using your library (which is wonderful by the way) and I run into a race condition with -race on BuildV3Model in my tests

func LoadOpenAPIModel(openAPIRoot string, openAPIPath string, opts *DocOptions) (*libopenapi.DocumentModel[v3.Document], map[string]*yaml.Node, error) {
	data, errRead := os.ReadFile(openAPIPath)
	if errRead != nil {
		return nil, nil, fmt.Errorf("cannot read file: %e", errRead)
	}

	dir := filepath.Dir(openAPIPath)
	// the CWD when doc is created is the scope in which the refs can be located
	// so if refs are located in the parent directory of the spec, we need to change the CWD
	// to the parent directory.
	if errChdir := os.Chdir(openAPIRoot); errChdir != nil {
		return nil, nil, fmt.Errorf("cannot change directory to %s: %e", openAPIRoot, errChdir)
	}
	config := datamodel.DocumentConfiguration{
		AllowFileReferences: true,
		BasePath:            dir,
		// Avoids a data race condition
		ExtractRefsSequentially: true,
	}

	if len(opts.IncludeTags) > 0 {
		data = onlyIncludeTags(data, opts.IncludeTags)
	}
	// We can't use github.com/pb33f/libopenapi/bundler#BundleBytes here to simply bundle
	// everything into a single file for ease of transform because it also
	// inlines most schemas, but the openapi generator requires schemas like enums
	// to be separate named schema objects, otherwise they get interpreted as
	// simple strings.
	document, errNewDoc := libopenapi.NewDocumentWithConfiguration(data, &config)

	if errNewDoc != nil {
		return nil, nil, fmt.Errorf("cannot create new document: %e", errNewDoc)
	}
	//document.GetRolodex().AddLocalFS(dir, fs.NewGlobalFS(dir))
	docModel, errV3 := document.BuildV3Model()
	if len(errV3) > 0 {
		for i := range errV3 {
			fmt.Printf("error: %e\n", errV3[i])
		}
		return nil, nil, fmt.Errorf("cannot create v3 model from document: %d errors reported", len(errV3))
	}

	refFiles := map[string]*yaml.Node{}

	// gets files linked in $refs, includes indirect references
	for _, index := range docModel.Index.GetRolodex().GetIndexes() {
		refFileBytes, _ := os.ReadFile(index.GetSpecAbsolutePath())
		var refFileNode yaml.Node
		if errUnmarshal := yaml.Unmarshal(refFileBytes, &refFileNode); errUnmarshal != nil {
			return nil, nil, fmt.Errorf("cannot unmarshal ref file: %e", errUnmarshal)
		}
		refFiles[index.GetSpecAbsolutePath()] = &refFileNode
	}
	return docModel, refFiles, nil
}

Based on what I can tell, with

ExtractRefsSequentially: false

A go thread is spawned for each ref. I assume that's because for remote refs and stuff like the comment inside mentions, it can take a bit to resolve.

Goroutine 25 (finished) created at:
  github.com/pb33f/libopenapi/index.(*SpecIndex).ExtractComponentsFromRefs()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/vendor/github.com/pb33f/libopenapi/index/extract_refs.go:683 +0x3b0
  github.com/pb33f/libopenapi/index.createNewIndex()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/vendor/github.com/pb33f/libopenapi/index/spec_index.go:90 +0x40c
  github.com/pb33f/libopenapi/index.NewSpecIndexWithConfig()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/vendor/github.com/pb33f/libopenapi/index/spec_index.go:50 +0x590
  github.com/pb33f/libopenapi/index.(*Rolodex).IndexTheRolodex()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/vendor/github.com/pb33f/libopenapi/index/rolodex.go:314 +0xf58
  github.com/pb33f/libopenapi/datamodel/low/v3.createDocument()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/vendor/github.com/pb33f/libopenapi/datamodel/low/v3/create_document.go:104 +0x8c8
  github.com/pb33f/libopenapi/datamodel/low/v3.CreateDocumentFromConfig()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/vendor/github.com/pb33f/libopenapi/datamodel/low/v3/create_document.go:28 +0x4c4
  github.com/pb33f/libopenapi.(*document).BuildV3Model()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/vendor/github.com/pb33f/libopenapi/document.go:314 +0x498
  code.hq.twilio.com/twilio/rest-proxy/rest-proxy-preprocessor/lib/util.LoadOpenAPIModel()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/lib/util/openapi.go:147 +0x2a4
  code.hq.twilio.com/twilio/rest-proxy/rest-proxy-preprocessor/cmd/transform.GetCmd.func1()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/cmd/transform/transform.go:44 +0x1f0
  github.com/spf13/cobra.(*Command).execute()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/vendor/github.com/spf13/cobra/command.go:987 +0xc24
  github.com/spf13/cobra.(*Command).ExecuteC()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/vendor/github.com/spf13/cobra/command.go:1115 +0x4b8
  github.com/spf13/cobra.(*Command).Execute()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/vendor/github.com/spf13/cobra/command.go:1039 +0x31c
  code.hq.twilio.com/twilio/rest-proxy/rest-proxy-preprocessor/cmd/transform.Test_getCmdTransform()
      /Users/<name>/Documents/edge-projects/rest-proxy/rest-proxy-preprocessor/cmd/transform/transform_test.go:26 +0x320
  testing.tRunner()
      /nix/store/460vdyz0ghxh8n5ibq3fgc3s63is68cd-go-1.22.2/share/go/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /nix/store/460vdyz0ghxh8n5ibq3fgc3s63is68cd-go-1.22.2/share/go/src/testing/testing.go:1742 +0x40

My assumption since it only seems to occur with sequential tests is that there are lingering go threads spawned by the ref resolver, and these persist between function runs. I wanted to make a PR to possibly fix it, but I wasn't sure what the intended behavior was or if we were using it wrong. I was thinking two possible solutions would be to expose a channel to wait on, or make a sister function that took in a context that we could cancel between tests or wait on.