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

Deadlocks in Execute #31

Closed
benlife5 opened this issue Nov 26, 2024 · 9 comments
Closed

Deadlocks in Execute #31

benlife5 opened this issue Nov 26, 2024 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@benlife5
Copy link

Hi,

We've occasionally been seeing deadlocks in the Execute function. Seems to block the timeout functionality from triggering too. Unfortunately, I don't have a way to reproduce, but I've added additional logging for when it happens and will update this issue if I get more details.

Our usage:

func (gen *SassCompiler) Compile(scss string) (string, error) {
	if len(scss) > 0 {
		res, err := transpiler.Execute(godartsass.Args{
			Source:         scss,
			OutputStyle:    godartsass.OutputStyleCompressed,
			ImportResolver: scssResolver{},
		})
		if err != nil {
			return "", yerrors.Wrap(err)
		}
		return strings.TrimPrefix(res.CSS, "\ufeff"), nil
	}

	return "", nil
}

Relevant part of the goroutine trace:

1367 @ ...
#   0x474724    sync.runtime_SemacquireMutex+0x24                                       GOROOT/src/runtime/sema.go:77
#   0x48421c    sync.(*Mutex).lockSlow+0x15c                                            GOROOT/src/sync/mutex.go:171
#   0x22da651   sync.(*Mutex).Lock+0x71                                             GOROOT/src/sync/mutex.go:90
#   0x22da626   github.com/bep/godartsass/v2.(*Transpiler).sendInboundMessage+0x46                      external/com_github_bep_godartsass_v2/transpiler.go:506
#   0x22da544   github.com/bep/godartsass/v2.(*Transpiler).newCall+0x2a4                            external/com_github_bep_godartsass_v2/transpiler.go:502
#   0x22d8904   github.com/bep/godartsass/v2.(*Transpiler).Execute+0xa4                             external/com_github_bep_godartsass_v2/transpiler.go:227
#   0x22e29f7   path/to/scsscompiler/lpsass.(*SassCompiler).Compile+0xd7             path/to/scsscompiler/lpsass/libsass.go:79

Occurred with version 2.0.0, I've just upgraded to 2.3.0 and will update if it happens again

@bep bep added the bug Something isn't working label Nov 27, 2024
@bep bep self-assigned this Nov 27, 2024
bep added a commit that referenced this issue Nov 27, 2024
@bep
Copy link
Owner

bep commented Nov 27, 2024

Thanks for the report. I'm assuming #32 will fix this, but I will leave this issue open. Note that this will not fix the underlying issue (a panic?).

bep added a commit that referenced this issue Nov 27, 2024
@benlife5
Copy link
Author

benlife5 commented Dec 2, 2024

Thanks for the quick fix! We'll update and monitor

@benlife5
Copy link
Author

benlife5 commented Dec 5, 2024

@bep I've been running into issues updating the package. Is it possible that go.mod should have been updated to require a later version of google.golang.org/protobuf as part of #28? It looks like protobuf removed that panic after 1.30 and internal/embeddedsass/embedded_sass.pb.go was generated with 1.35.

This is the panic I'm getting:

panic: not supported

goroutine 60 [running]:
google.golang.org/protobuf/internal/impl.Export.MessageStateOf({}, {0xc00132db10?, 0x474613?})
	external/org_golang_google_protobuf/internal/impl/pointer_reflect.go:198 +0x25
github.com/bep/godartsass/v2/internal/embeddedsass.(*InboundMessage).ProtoReflect(0x415354?)
	external/com_github_bep_godartsass_v2/internal/embeddedsass/embedded_sass.pb.go:458 +0x3c```

@bep
Copy link
Owner

bep commented Dec 6, 2024

Is it possible that go.mod should have been updated to require a later version of google.golang.org/protobuf as part of #28?

I don't know, but I could/should certainly bump that/the dependenccies. I will do that, thanks for the input.

@bep
Copy link
Owner

bep commented Dec 6, 2024

@benlife5
Copy link
Author

benlife5 commented Dec 9, 2024

Thanks, I was able to upgrade to the new release smoothly

@bep
Copy link
Owner

bep commented Dec 9, 2024

@benlife5 does this mean that this issue is resolved and can be closed?

@benlife5
Copy link
Author

I think so. The deadlocks occurs infrequently so I can't say for certain that it is no longer happening but I'm good with closing this item and I can reopen if it recurs.

@samuel-280589
Copy link

#31 (comment)

@bep bep closed this as completed Dec 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants