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

Crash from Sentinel issue on arm 32-bit #1033

Closed
kb2ma opened this issue Jul 21, 2021 · 14 comments · Fixed by alibaba/sentinel-golang#422 or #1045
Closed

Crash from Sentinel issue on arm 32-bit #1033

kb2ma opened this issue Jul 21, 2021 · 14 comments · Fixed by alibaba/sentinel-golang#422 or #1045
Assignees
Labels
do-not-merge PR is not ready for merging good first issue Good for newcomers P1 size/XS 2 days of work triaged/resolved
Milestone

Comments

@kb2ma
Copy link

kb2ma commented Jul 21, 2021

In what area(s)?

/area runtime

What version of Dapr?

1.2.2

Expected Behavior

No crash at startup.

Actual Behavior

dapr v1.2.2 crashes at startup on arm 32-bit due to an integer overflow issue in Sentinel v1.0.2, as I reported upstream, including a stack trace. What's odd is that I have not configured dapr to actually use Sentinel -- no component or configuration yaml.

I have found two ways to workaround the error. First, comment out references to Sentinel in cmd/daprd/main.go. Second, use a version of Sentinel patched for the overflow. See Sentinel patch and use.

Steps to Reproduce the Problem

You should be able to reproduce just by running an app with dapr v1.2.2 on an arm 32-bit platform, like a Raspberry Pi 3.

Release Note

RELEASE NOTE: UPDATE Runtime dependency.

@daixiang0
Copy link
Member

Do you have interested to fix it?

@kb2ma
Copy link
Author

kb2ma commented Jul 23, 2021

Do you have interested to fix it?

I would prefer not to work with the upstream Sentinel repository because I have no experience with it and don't need to use it. This work would take time that I really don't have ATM.

It does seem odd that just compiling dapr with the Sentinel plugin present but not configured would cause this issue in dapr. Once again though I don't need to use Sentinel and have a working solution, and unfortunately no time to pursue it.

@yaron2
Copy link
Member

yaron2 commented Jul 23, 2021

/cc @artursouza @wcs1only

@artursouza artursouza transferred this issue from dapr/dapr Jul 23, 2021
@artursouza artursouza added do-not-merge PR is not ready for merging good first issue Good for newcomers P1 size/XS 2 days of work triaged/resolved labels Jul 23, 2021
@artursouza artursouza added this to the v1.4 milestone Jul 23, 2021
@daixiang0
Copy link
Member

Seems to need a fix in upstream, just use this patch directly.

@daixiang0
Copy link
Member

/assign

@yaron2 if not emergency, will wait for next release of upstream.

@kb2ma
Copy link
Author

kb2ma commented Jul 28, 2021

Not an emergency from my perspective. Thanks for resolving.

@artursouza
Copy link
Member

Assigned to @daixiang0

@artursouza
Copy link
Member

@daixiang0 Can you test it on an ARM32 bits.

@daixiang0
Copy link
Member

@artursouza I am afraid that I can not since no ARM32 machine, just run it within hacked container.

@artursouza
Copy link
Member

artursouza commented Jul 30, 2021

@daixiang0 No worries. Can you send a PR?

@kb2ma Can you validate this once the fix is in master?

@kb2ma
Copy link
Author

kb2ma commented Aug 1, 2021

@artursouza Sure, I'd be happy to test.

@Oceanswave
Copy link

Oceanswave commented Aug 21, 2021

Seems like this was fixed, but is once again breaking on :latest and :edge-linux-arm

goroutine 1 [running]:
github.com/alibaba/sentinel-golang/core/stat/base.NewAtomicBucketWrapArrayWithTime(0x14, 0x1f4, 0x660994d5, 0x17b, 0x314f2b8, 0x51b1f80, 0x51b1f80)
        /home/runner/go/pkg/mod/github.com/alibaba/sentinel-golang@v1.0.2/core/stat/base/leap_array.go:83 +0x334
github.com/alibaba/sentinel-golang/core/stat/base.NewAtomicBucketWrapArray(0x14, 0x1f4, 0x314f2b8, 0x51b1f80, 0x0)
        /home/runner/go/pkg/mod/github.com/alibaba/sentinel-golang@v1.0.2/core/stat/base/leap_array.go:108 +0x48
github.com/alibaba/sentinel-golang/core/stat/base.NewBucketLeapArray(0x14, 0x2710, 0xb6f1ccb8)
        /home/runner/go/pkg/mod/github.com/alibaba/sentinel-golang@v1.0.2/core/stat/base/bucket_leap_array.go:60 +0x84
github.com/alibaba/sentinel-golang/core/stat.NewBaseStatNode(0x2, 0x3e8, 0x4ed0d74)
        /home/runner/go/pkg/mod/github.com/alibaba/sentinel-golang@v1.0.2/core/stat/base_node.go:36 +0x30
github.com/alibaba/sentinel-golang/core/stat.NewResourceNode(0x2bc34cb, 0x19, 0x0, 0x1c920)
        /home/runner/go/pkg/mod/github.com/alibaba/sentinel-golang@v1.0.2/core/stat/resource_node.go:32 +0x34
github.com/alibaba/sentinel-golang/core/stat.init()
        /home/runner/go/pkg/mod/github.com/alibaba/sentinel-golang@v1.0.2/core/stat/node_storage.go:27 +0x2c

@artursouza
Copy link
Member

@daixiang0 Can this be fixed again and validated as part of the build? I see that we are skipping running tests in arm32.

@artursouza
Copy link
Member

@kb2ma Please, validate the current master version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge PR is not ready for merging good first issue Good for newcomers P1 size/XS 2 days of work triaged/resolved
Projects
None yet
5 participants