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.

Sign up for GitHub

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

Jump to bottom

Nil pointer panic in sdk. #11727

Closed
rshade opened this issue Dec 22, 2022 · 8 comments
Closed

Nil pointer panic in sdk. #11727

rshade opened this issue Dec 22, 2022 · 8 comments
Labels
area/sdks Pulumi language SDKs kind/bug Some behavior is incorrect or out of spec language/go p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Milestone
0.83

Comments

@rshade
Copy link
Contributor

rshade commented Dec 22, 2022 •
edited by squaremo

We are getting the following error, and it is panic'ing pulumi cli.

         [stdout]          Diagnostics:
         [stdout]            pulumi:pulumi:Stack (<REDACTED>):
         [stdout]              panic: runtime error: invalid memory address or nil pointer dereference
         [stdout]              [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xb6cd78]
         [stdout]              goroutine 1798 [running]:
         [stdout]              github.com/pulumi/pulumi/sdk/v3/go/pulumi.mergeDependencies({0x0, 0x1, 0xc000570000?}, {0xc000791a10, 0x1, 0x40c0ad?})
         [stdout]              	/home/nonroot/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.39.0/go/pulumi/types.go:212 +0x218
         [stdout]              github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).fulfillValue(0xc0005d60e0, {0x230f020?, 0xc000791a00?, 0x0?}, 0x1, 0x0, {0xc000791a10, 0x1, 0x1}, {0x0, ...})
         [stdout]              	/home/nonroot/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.39.0/go/pulumi/types.go:198 +0x39e
         [stdout]              github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).ApplyTWithContext.func1()
         [stdout]              	/home/nonroot/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.39.0/go/pulumi/types.go:518 +0x61b
         [stdout]              created by github.com/pulumi/pulumi/sdk/v3/go/pulumi.(*OutputState).ApplyTWithContext
         [stdout]              	/home/nonroot/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.39.0/go/pulumi/types.go:495 +0x411
         [stdout]              exit status 2
         [stdout]           
         [stdout]              error: an unhandled error occurred: waiting for RPCs: rpc error: code = Unavailable desc = error reading from server: EOF

We believe it has to do with the following line:
https://github.com/pulumi/pulumi/blob/v3.39.0/sdk/go/pulumi/types.go#L212
@lukehoban lukehoban added area/sdks Pulumi language SDKs language/go needs-triage Needs attention from the triage team labels Dec 22, 2022
@Zaid-Ajaj
Copy link
Contributor

Hi @rshade, this looks interesting because the expression range <slice> will not error if <slice> is nil so it must be something else. Can you please provide a piece of code so we can reproduce the problem?

@Zaid-Ajaj Zaid-Ajaj added kind/bug Some behavior is incorrect or out of spec and removed needs-triage Needs attention from the triage team labels Dec 27, 2022
@squaremo
Copy link
Contributor

squaremo commented Jan 5, 2023

What version of the CLI is in question?

@Frassle
Copy link
Member

Frassle commented Jan 5, 2023

It's in the go sdk, not the cli.

@squaremo
Copy link
Contributor

squaremo commented Jan 5, 2023

It's in the go sdk, not the cli.

OK, what is the version of the SDK? The code snippet link is to a particular commit; I want to establish whether that is the code at the version used.

@justinvp
Copy link
Member

justinvp commented Jan 5, 2023

The version is in the stack trace: @v3.39.0

@squaremo
Copy link
Contributor

squaremo commented Jan 5, 2023

The version is in the stack trace: @v3.39.0

So it is, I didn't notice that. I've edited the description so that it explicitly points at the code as at the version used (the line is the same -- @rshade if you deliberately used the SHA1 so it would show a preview, apologies, my change has lost that).

@t0yv0 t0yv0 mentioned this issue Jan 5, 2023
Merged
3 tasks
bors bot added a commit that referenced this issue Jan 5, 2023
@bors @t0yv0
Merge #11776
ae9f0fd 
11776: Make sure output state deps is only accessed under a lock r=t0yv0 a=t0yv0

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

Related to #11727 though that might have possibly been fixed on master already, as the last known failing version is  3.43.1

Fixes # (issue)

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


Co-authored-by: Anton Tayanovskyy <anton@pulumi.com>
@t0yv0
Copy link
Member

t0yv0 commented Jan 6, 2023

The Pulumi Platform team took a deep dive on the issue yesterday and I want to summarize it in a quick update.

The issue arises from a race leading to a torn slice being passed into mergeDependencies as the first argument (len(x) == 1 && x == nil).

Since v3.39.0 there was we had work done to pass the test suite under the race detector, which introduced locks around the code in question. It is our understanding that this issue should be fixed in version v3.46.1 and higher.

We have confidence but not 100% certainty here that the issue is fixed because we failed to construct a repro case - a solid repro would definitely be highly appreciated! To mitigate any further issues, we introduced checks that detect memory corruption earlier and produce better panic stack traces in case it happens so that it is easier to reproduce the issue. This work is #11776 and #11779

@abhinav abhinav mentioned this issue Jan 6, 2023
Closed
@joeduffy joeduffy added the p1 A bug severe enough to be the next item assigned to an engineer label Jan 7, 2023
@justinvp justinvp added the resolution/fixed This issue was fixed label Jan 8, 2023
@justinvp
Copy link
Member

justinvp commented Jan 8, 2023

Closing since we believe the issue has been fixed since v3.46.1.

@justinvp justinvp closed this as completed Jan 8, 2023
@mikhailshilkov mikhailshilkov added this to the 0.83 milestone Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/sdks Pulumi language SDKs kind/bug Some behavior is incorrect or out of spec language/go p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Projects
None yet
Milestone
0.83
Development

No branches or pull requests
9 participants
@t0yv0 @squaremo @rshade @lukehoban @justinvp @Frassle @mikhailshilkov @joeduffy @Zaid-Ajaj