Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make OutputState pass the -race detector #11248

Merged
merged 1 commit into from Nov 3, 2022

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented Nov 3, 2022

This is the last step necessary to allow pulumi-yaml to run a subset of its tests with -race=true.

The race is is between ApplyT and fulfillValue on copying a slice.

Both me (@iwahbe) and @t0yv0 have independently confirmed that the race is not hiding a deeper issue. Further discussion of the issue is at #11186.

A stack trace from the -race flag that this issue fixes:

WARNING: DATA RACE
Write at 0x00c000422ec0 by goroutine 463:
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).fulfillValue()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/types.go:198 +0x590
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).ApplyTWithContext.func1()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/types.go:516 +0x8a5

Previous read at 0x00c000422ec0 by goroutine 306:
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).dependencies()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/types.go:136 +0x406
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).ApplyTWithContext()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/types.go:492 +0x14a
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).ApplyT()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/types.go:440 +0xb8
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.AnyOutput.ApplyT()
      <autogenerated>:1 +0x49
  github.com/pulumi/pulumi-yaml/pkg/pulumiyaml.TestSplit.func1.1()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi-yaml/pkg/pulumiyaml/run_test.go:890 +0x1d3
  github.com/pulumi/pulumi-yaml/pkg/pulumiyaml.testTemplateDiags.func2()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi-yaml/pkg/pulumiyaml/run_test.go:286 +0x516
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.RunWithContext()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/run.go:120 +0x258
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.runErrInner()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/run.go:96 +0x4b1
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.RunErr()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/run.go:63 +0x23a
  github.com/pulumi/pulumi-yaml/pkg/pulumiyaml.testTemplateDiags()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi-yaml/pkg/pulumiyaml/run_test.go:274 +0x2e
  github.com/pulumi/pulumi-yaml/pkg/pulumiyaml.testTemplate()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi-yaml/pkg/pulumiyaml/run_test.go:335 +0x44
  github.com/pulumi/pulumi-yaml/pkg/pulumiyaml.TestSplit.func1()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi-yaml/pkg/pulumiyaml/run_test.go:886 +0x2f5
  testing.tRunner()
      /usr/local/Cellar/go/1.19.2/libexec/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /usr/local/Cellar/go/1.19.2/libexec/src/testing/testing.go:1493 +0x47

@iwahbe iwahbe self-assigned this Nov 3, 2022
@pulumi-bot
Copy link
Contributor

Changelog

[uncommitted] (2022-11-03)

@iwahbe iwahbe added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Nov 3, 2022
@iwahbe
Copy link
Member Author

iwahbe commented Nov 3, 2022

bors r+

bors bot added a commit that referenced this pull request Nov 3, 2022
11248: Make OutputState pass the `-race` detector r=iwahbe a=iwahbe

This is the last step necessary to allow pulumi-yaml to run a subset of its tests with `-race=true`. 

The race is is between `ApplyT` and `fulfillValue` on copying a slice. 

Both me (`@iwahbe)` and `@t0yv0` have independently confirmed that the race is not hiding a deeper issue. Further discussion of the issue is at #11186.

A stack trace from the `-race` flag that this issue fixes:
```
WARNING: DATA RACE
Write at 0x00c000422ec0 by goroutine 463:
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).fulfillValue()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/types.go:198 +0x590
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).ApplyTWithContext.func1()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/types.go:516 +0x8a5

Previous read at 0x00c000422ec0 by goroutine 306:
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).dependencies()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/types.go:136 +0x406
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).ApplyTWithContext()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/types.go:492 +0x14a
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).ApplyT()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/types.go:440 +0xb8
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.AnyOutput.ApplyT()
      <autogenerated>:1 +0x49
  github.com/pulumi/pulumi-yaml/pkg/pulumiyaml.TestSplit.func1.1()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi-yaml/pkg/pulumiyaml/run_test.go:890 +0x1d3
  github.com/pulumi/pulumi-yaml/pkg/pulumiyaml.testTemplateDiags.func2()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi-yaml/pkg/pulumiyaml/run_test.go:286 +0x516
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.RunWithContext()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/run.go:120 +0x258
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.runErrInner()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/run.go:96 +0x4b1
  github.com/pulumi/pulumi/sdk/v3/go/pulumi.RunErr()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi/sdk/go/pulumi/run.go:63 +0x23a
  github.com/pulumi/pulumi-yaml/pkg/pulumiyaml.testTemplateDiags()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi-yaml/pkg/pulumiyaml/run_test.go:274 +0x2e
  github.com/pulumi/pulumi-yaml/pkg/pulumiyaml.testTemplate()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi-yaml/pkg/pulumiyaml/run_test.go:335 +0x44
  github.com/pulumi/pulumi-yaml/pkg/pulumiyaml.TestSplit.func1()
      /Users/ianwahbe/go/src/github.com/pulumi/pulumi-yaml/pkg/pulumiyaml/run_test.go:886 +0x2f5
  testing.tRunner()
      /usr/local/Cellar/go/1.19.2/libexec/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /usr/local/Cellar/go/1.19.2/libexec/src/testing/testing.go:1493 +0x47
```

Co-authored-by: Ian Wahbe <ian@wahbe.com>
@bors
Copy link
Contributor

bors bot commented Nov 3, 2022

Build failed:

@iwahbe
Copy link
Member Author

iwahbe commented Nov 3, 2022

bors retry

@bors
Copy link
Contributor

bors bot commented Nov 3, 2022

Build succeeded:

@bors bors bot merged commit 6d85084 into master Nov 3, 2022
@pulumi-bot pulumi-bot deleted the iwahbe/output-pass-race-detector branch November 3, 2022 22:51
@t0yv0 t0yv0 mentioned this pull request Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants