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

[test]: panic: Log in goroutine after ... has completed: Starting main loop #4576

Closed
kennyaz opened this issue Jul 11, 2023 · 2 comments
Closed

[test]: panic: Log in goroutine after ... has completed: Starting main loop #4576

kennyaz opened this issue Jul 11, 2023 · 2 comments
Labels
bug help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@kennyaz
Copy link
Contributor

kennyaz commented Jul 11, 2023

What happened?

panic: Log in goroutine after ... has completed: Starting main loop

Steps to reproduce

Not Applicable

Expected behavior

There is a possibility that the goroutine is not finished when Close is called since there is no waitGroup that is waiting for the c.logger.Info("Starting main loop") to be done executing before the application is shutdown

Relevant log output

panic: Log in goroutine after ... has completed:  Starting main loop

goroutine 39 [running]:
testing.(*common).logDepth(0xc00052bd40, {0xc0001b95f0, 0x30}, 0x3)
        GOROOT/src/testing/testing.go:1003 +0x4e7
testing.(*common).log(...)
        GOROOT/src/testing/testing.go:985
testing.(*common).Logf(0xc00052bd40, {0x5e4ddb?, 0x14aceb7?}, {0xc0000401c0?, 0xffffffffffffffff?, 0x31?})
        GOROOT/src/testing/testing.go:1036 +0x5a
go.uber.org/zap/zaptest.testingWriter.Write({{0x8eecf0?, 0xc00052bd40?}, 0x0?}, {0xc000241400?, 0x31, 0xc0000401b0?})
        external/org_uber_go_zap/zaptest/logger.go:130 +0xe6
go.uber.org/zap/zapcore.(*ioCore).Write(0xc0001cd860, {0x0, {0xc1233bb7a60da4ca, 0x1863da2, 0x2bf1e00}, {0x0, 0x0}, {0x5fa77a, 0x12}, {0x0, ...}, ...}, ...)
        external/org_uber_go_zap/zapcore/core.go:99 +0xb5
go.uber.org/zap/zapcore.(*CheckedEntry).Write(0xc0001b71e0, {0x0, 0x0, 0x0})
        external/org_uber_go_zap/zapcore/entry.go:255 +0x1d9
go.uber.org/zap.(*Logger).Info(0x0?, {0x5fa77a?, 0x0?}, {0x0, 0x0, 0x0})
        external/org_uber_go_zap/logger.go:220 +0x59
github.com/jaegertracing/jaeger/cmd/ingester/app/consumer.(*Consumer).Start.func1()
        external/com_github_jaegertracing_jaeger/cmd/ingester/app/consumer/consumer.go:79 +0x4b
created by github.com/jaegertracing/jaeger/cmd/ingester/app/consumer.(*Consumer).Start
        external/com_github_jaegertracing_jaeger/cmd/ingester/app/consumer/consumer.go:78 +0x6a

Screenshot

No response

Additional context

No response

Jaeger backend version

No response

SDK

No response

Pipeline

No response

Stogage backend

No response

Operating system

No response

Deployment model

No response

Deployment configs

No response
@kennyaz kennyaz added the bug label Jul 11, 2023
@yurishkuro yurishkuro changed the title [Bug]: [test]: panic: Log in goroutine after ... has completed: Starting main loop Jul 11, 2023
@yurishkuro yurishkuro added the help wanted Features that maintainers are willing to accept but do not have cycles to implement label Jul 11, 2023
@yurishkuro
Copy link
Member

which test is that?

@kennyaz
Copy link
Contributor Author

kennyaz commented Jul 14, 2023

It is the test on our side. In our code, we have a test that basically call the Start function and Close function with fxtest.

@kennyaz kennyaz mentioned this issue Jul 14, 2023
Merged
yurishkuro pushed a commit that referenced this issue Jul 14, 2023
@kennyaz
[kafka-consumer] Use wait group to ensure goroutine is finished befor…
673b69f 
…e returning from Close (#4582)

## Which problem is this PR solving?
Resolves # [4576](#4576)

## Short description of the changes
Once we call
[start](https://github.com/jaegertracing/jaeger/blob/main/cmd/ingester/app/consumer/consumer.go#L76)
and call
[close](https://github.com/jaegertracing/jaeger/blob/main/cmd/ingester/app/consumer/consumer.go#L76)
again with fxtest module, there is a chance that the application will
close the goroutine before the
l[ogline](https://github.com/jaegertracing/jaeger/blob/main/cmd/ingester/app/consumer/consumer.go#L79)
is executed causing **panic Log in routine after .. has completed**.

Fix:
Add a wait group that will wait for the goroutine to finish before
closing the channel.

---------

Signed-off-by: kennyaz <[email protected]>
@kennyaz kennyaz closed this as completed Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@yurishkuro @kennyaz