Is NewBlock save to use concurrently

[abergmeier]

I wonder why calling NewBlock seems to trigger race detection.

Expected behavior

Everything should work fine.

Actual behavior

Golang race detector complains:

WARNING: DATA RACE
Write at 0x0000009f7640 by goroutine 15:
  github.com/hashicorp/hcl/v2/hclwrite.formatSpaces()
      /home/abergmei/go/pkg/mod/github.com/hashicorp/hcl/[email protected]/hclwrite/format.go:127 +0x1ca
  github.com/hashicorp/hcl/v2/hclwrite.format()
      /home/abergmei/go/pkg/mod/github.com/hashicorp/hcl/[email protected]/hclwrite/format.go:36 +0xa6
  github.com/hashicorp/hcl/v2/hclwrite.TokensForValue()
      /home/abergmei/go/pkg/mod/github.com/hashicorp/hcl/[email protected]/hclwrite/generate.go:25 +0xa5
  github.com/hashicorp/hcl/v2/hclwrite.(*Block).init()
      /home/abergmei/go/pkg/mod/github.com/hashicorp/hcl/[email protected]/hclwrite/ast_block.go:39 +0x70c
  github.com/hashicorp/hcl/v2/hclwrite.NewBlock()
      /home/abergmei/go/pkg/mod/github.com/hashicorp/hcl/[email protected]/hclwrite/ast_block.go:29 +0x179
  ...

Previous write at 0x0000009f7640 by goroutine 14:
  github.com/hashicorp/hcl/v2/hclwrite.formatSpaces()
      /home/abergmei/go/pkg/mod/github.com/hashicorp/hcl/[email protected]/hclwrite/format.go:127 +0x1ca
  github.com/hashicorp/hcl/v2/hclwrite.format()
      /home/abergmei/go/pkg/mod/github.com/hashicorp/hcl/[email protected]/hclwrite/format.go:36 +0xa6
  github.com/hashicorp/hcl/v2/hclwrite.TokensForValue()
      /home/abergmei/go/pkg/mod/github.com/hashicorp/hcl/[email protected]/hclwrite/generate.go:25 +0xa5
  github.com/hashicorp/hcl/v2/hclwrite.(*Block).init()
      /home/abergmei/go/pkg/mod/github.com/hashicorp/hcl/[email protected]/hclwrite/ast_block.go:39 +0x70c
  github.com/hashicorp/hcl/v2/hclwrite.NewBlock()
      /home/abergmei/go/pkg/mod/github.com/hashicorp/hcl/[email protected]/hclwrite/ast_block.go:29 +0x179
  ...

From the code it seems like this may happen if the format lines get shared. I however could not find such a package variable yet.

Steps to reproduce

1. Start two goroutines
2. Have functions call hclwrite.NewBlock in both of them

[jub0bs]

@abergmeier Is this data race reproducible after https://github.com/hashicorp/hcl/pull/511?

[apparentlymart]

Speaking generally, the data structures in hclwrite have no internal synchronization and so if you intend to access the same object concurrently from multiple goroutines then you will need to provide your own outer synchronization in a similar way as you would for Go's built-in data structures.

Of course that general rule should not affect a call to NewBlock, because that is constructing an entirely new object that should not be aliased anywhere until that function returns. It should be safe to call NewBlock itself concurrently from multiple goroutines, as long as you don't subsequently try to mutate the resulting block concurrently from multiple goroutines.

There was previously an accidentally-shared Token object used to represent the general absence of a token, which was the subject of https://github.com/hashicorp/hcl/pull/511 that @jub0bs mentioned. That caused problems because the "formatting" functionality works by modifying the SpacesBefore field of each token in-place, and so it fundamentally assumes that each distinct token object appears in at most one location in the internal syntax tree. As far as I know that particular problem has been fixed in main, but the version number in the stack trace in this issue (2.6.0) is four and half years old, so it would not have that fix, and so I think that old bug is the most likely explanation for the behavior you observed.