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

[bug]: ChannelRouter cannot be shutdown while the syncGraphWithChain function is running. #8721

Open
ViktorTigerstrom opened this issue May 3, 2024 · 4 comments · May be fixed by #8497
Open
Assignees
Labels
bug Unintended code behaviour
Milestone

Comments

@ViktorTigerstrom
Copy link
Collaborator

ViktorTigerstrom commented May 3, 2024

Background

The syncGraphWithChain function in routing/router.go cannot be stopped by a shutdown signal while it's running. The function is ment to be able to exit early by closing the quit channel here:

case <-r.quit:

But as the syncGraphWithChain function is synchronously called in the Start function, the Stop function which closes the quit channel, can never be triggered until the syncing is complete. The only call-sites for the Stop function can only ever happen after the ChannelRouter Start function has fully executed:

lnd/server.go

Line 2020 in fb632bb

cleanup = cleanup.add(s.chanRouter.Stop)
&

lnd/lnd.go

Line 681 in fb632bb

defer server.Stop()

Steps to reproduce

To reproduce the issue, you can add some simple code to the beginning of the syncGraphWithChain function like the following:

for i := 0; i <= 10000; i++ {
    time.Sleep(3 * time.Millisecond)
    
    log.Infof("Syncing test: %v", i)
    
    select {
    case <-r.quit:
	    return ErrRouterShuttingDown
    default:
    }
}

Then try to shutdown lnd with a shutdown signal while the code above is executing.

Expected behaviour

The syncGraphWithChain function should exit early by a shutdown signal.

Actual behaviour

lnd will only shutdown after the syncGraphWithChain and the Start function has finished their execution.

@ViktorTigerstrom ViktorTigerstrom added bug Unintended code behaviour needs triage labels May 3, 2024
@ViktorTigerstrom ViktorTigerstrom changed the title [bug]: ChannelRouter cannot be shutdown while the syncGraphWithChain is running. [bug]: ChannelRouter cannot be shutdown while the syncGraphWithChain function is running. May 3, 2024
@yyforyongyu
Copy link
Collaborator

Think this will be fixed by #8497 cc @ziggie1984

@ziggie1984
Copy link
Collaborator

Yes as soon as we always do the cleanup we are fine and close the stop channel when triggering the shutdown.

@ViktorTigerstrom
Copy link
Collaborator Author

ViktorTigerstrom commented May 7, 2024

Awesome, thanks! I just want to clarify though, that the ChannelRouter Start function will not return while the syncGraphWithChain function is running. So only adding the Stop function for the ChannelRouter to the cleanUp func in the server, before the Start function is executed won't really change the behaviour detailed in this issue unless I'm missing something?

@ziggie1984
Copy link
Collaborator

You are right, thank you for the input will come up with a different solution.

@saubyk saubyk linked a pull request May 8, 2024 that will close this issue
@saubyk saubyk added this to the v0.18.1 milestone May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unintended code behaviour
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants