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

hugo server doesn't stop on Ctrl+C if theme name is misspelled after starting the server #8340

Closed
rootkea opened this issue Mar 18, 2021 · 7 comments

Comments

@rootkea
Copy link
Contributor

rootkea commented Mar 18, 2021

Hello!

hugo server says "Press Ctrl+C to stop" but the server doesn't stop if theme name is misspelled after starting the server. The only way to exit the hugo server process then is to close the terminal forcefully.

$ hugo version
hugo v0.81.0-59D15C97+extended linux/amd64 BuildDate=2021-02-19T17:07:12Z VendorInfo=gohugoio

Steps to reproduce:

  1. Start hugo server with correct theme: <theme_name> in config
  2. Now change <theme_name> to some non-existent theme name (e.g. misspelling)
  3. Now try to stop server with Ctrl+C
@rootkea rootkea changed the title Ctrl+C doesn't stop hugo server if theme names is misspelled after starting the server hugo server doesn't stop on Ctrl+C if theme names is misspelled after starting the server Mar 18, 2021
@rootkea
Copy link
Contributor Author

rootkea commented Mar 18, 2021

Doesn't matter even if the spelling is corrected later. Ctrl+C just doesn't stop the hugo server process.

@rootkea rootkea changed the title hugo server doesn't stop on Ctrl+C if theme names is misspelled after starting the server hugo server doesn't stop on Ctrl+C if theme name is misspelled after starting the server Mar 22, 2021
@davidofferman
Copy link

davidofferman commented Mar 22, 2021

I started looking into this. It started with version v0.80.0 with commit cea1574

Update:
It appears that when a full rebuild of the site occurs, the past Deps.BuildClosers never close. Running Deps.Close() before a site is built from hugo.buildSites() solves the issue.

It's likely a race condition, and I just noticed that Deps.Close is already deferred in HugoSites.Build

Update 2:
I'm pretty sure the issue is a deadlock in commandeerHugoState.hugo().

The method comandeer.fullRebuild() makes the channel c.created. This channel doesn't close in comandeer.loadConfig if hugolib.LoadConfig() fails.

@Vimux
Copy link

Vimux commented Apr 2, 2021

I (accidentally) reproduced this bug on my development machine this morning.
Not critical, but very annoying I would say. 😄

What version of Hugo are you using (hugo version)?

$ hugo version
hugo v0.82.0+extended darwin/amd64 BuildDate=unknown

Does this issue reproduce with the latest release?

Yes.

Steps to reproduce

  1. Start hugo server without errors
  2. Add ! sign as a line somewhere in config (more correct: add or remove something that would cause ERROR, for example, remove quotes where they are needed)
  3. Try to stop server with Ctrl+C

@danisztls
Copy link

danisztls commented Jul 8, 2021

I had a similar problem. It looks like any error that would result in a "fail to reload config" would break CTRL-C expected behavior.

Change of config file detected, rebuilding site.
2021-07-07 22:02:24.482 -0300
ERROR 2021/07/07 22:02:24 Failed to reload config:

ERROR 2021/07/07 22:02:24 Module "github.com/twbs/bootstrap" not found; either add it as a Hugo Module or store it in "/home/me/Dev/work/martin/symbols-doc/themes".: module does not exist
Rebuilt in 2 ms
^C^C^C^C^C^C^C^C^C^C^C^C
Change of config file detected, rebuilding site.
2021-07-07 22:02:59.482 -0300
Rebuilt in 886 ms
^C^C^C^C^C^C^C^C^Czsh: killed     hugo server --disableFastRender --environment testing

Version: 0.84.4-1 (Arch Linux)

@MasterGroosha
Copy link

MasterGroosha commented Nov 21, 2021

I'm having the same issue as @danisztls (Ctrl+C stops working on ANY error) with v0.89.4-AB01BA6E on Manjaro
The only way to stop this is to kill -9 <hugo server PID>, killing without -9 flag doesn't work as well

ptgott added a commit to ptgott/hugo that referenced this issue Mar 13, 2022
If a user starts hugo server and writes to the config file in a way
that renders it invalid, hugo server stops responding to SIGINTs,
meaning that a user needs to send a SIGKILL to the hugo process to stop
it.

*commandeer.serve needs to obtain a *hugolib.HugoSites in order to
close the Hugo environment. After handling signals/the stop channel,
*commandeer.serve calls c.hugo() to get the *hugolib.HugoSites. This
function receives from the c.created channel, which is closed when the
HugoSites is ready. However, if an error took place while loading the
config, c.created would never be closed, so Hugo would block on this
channel even after handling SIGINT.

The solution is to close c.created if we failed to load a config when
rebuilding the site, then check at the end of *commandeer.serve whether
the *hugolib.HugoSites is nil. If it is, we stop waiting for a HugoSites
to close, and exit the hugo process.

One issue that resulted from this fix was how to reset the c.created
channel during site rebuilds. In the current code, this is done by
assigning c.commandeerHugoState to a newCommandeerHugoState in
*commandeer.fullRebuild. However, this creates a race condition, since
other goroutines can be reading the value of c.created just as
*commandeer.fullRebuild is attempting to reassign it.

This change fixes the race condition by adding the lazy.Notifier type,
which preserves the use of c.created while allowing for it to be reset
in a goroutine-safe way. c.created is now a lazy.Notifier. I added this
type to the lazy package because it didn't seem worth it to add a new
package, but we can do this if it's cleaner. (I was thinking about using
sync.Cond for this, but the approach I used is closer to the original
use of the c.created channel and changes less code.)

Also adds an integration test for interrupting the server after fixing
an invalid configuration. An earlier version of this change caused hugo
server to stop exiting with a SIGINT after fixing a bad config, so I
wanted to ensure that we can prevent this.

Fixes gohugoio#8340
ptgott added a commit to ptgott/hugo that referenced this issue Mar 13, 2022
If a user starts hugo server and writes to the config file in a way
that renders it invalid, hugo server stops responding to SIGINTs,
meaning that a user needs to send a SIGKILL to the hugo process to stop
it.

*commandeer.serve needs to obtain a *hugolib.HugoSites in order to
close the Hugo environment. After handling signals/the stop channel,
*commandeer.serve calls c.hugo() to get the *hugolib.HugoSites. This
function receives from the c.created channel, which is closed when the
HugoSites is ready. However, if an error took place while loading the
config, c.created would never be closed, so Hugo would block on this
channel even after handling SIGINT.

The solution is to close c.created if we failed to load a config when
rebuilding the site, then check at the end of *commandeer.serve whether
the *hugolib.HugoSites is nil. If it is, we stop waiting for a HugoSites
to close, and exit the hugo process.

One issue that resulted from this fix was how to reset the c.created
channel during site rebuilds. In the current code, this is done by
assigning c.commandeerHugoState to a newCommandeerHugoState in
*commandeer.fullRebuild. However, this creates a race condition, since
other goroutines can be reading the value of c.created just as
*commandeer.fullRebuild is attempting to reassign it.

This change fixes the race condition by adding the lazy.Notifier type,
which preserves the use of c.created while allowing for it to be reset
in a goroutine-safe way. c.created is now a lazy.Notifier. I added this
type to the lazy package because it didn't seem worth it to add a new
package, but we can do this if it's cleaner. (I was thinking about using
sync.Cond for this, but the approach I used is closer to the original
use of the c.created channel.)

Also adds an integration test for interrupting the server after fixing
an invalid configuration. An earlier version of this change caused hugo
server to stop exiting with a SIGINT after fixing a bad config, so I
wanted to ensure that we can prevent this.

Fixes gohugoio#8340
ptgott added a commit to ptgott/hugo that referenced this issue Mar 15, 2022
If a user starts hugo server and writes to the config file in a way
that renders it invalid, hugo server stops responding to SIGINTs,
meaning that a user needs to send a SIGKILL to the hugo process to stop
it.

*commandeer.serve needs to obtain a *hugolib.HugoSites in order to
close the Hugo environment. After handling signals/the stop channel,
*commandeer.serve calls c.hugo() to get the *hugolib.HugoSites. This
function receives from the c.created channel, which is closed when the
HugoSites is ready. However, if an error took place while loading the
config, c.created would never be closed, so Hugo would block on this
channel even after handling SIGINT.

The solution is to close c.created if we failed to load a config when
rebuilding the site, then check at the end of *commandeer.serve whether
the *hugolib.HugoSites is nil. If it is, we stop waiting for a HugoSites
to close, and exit the hugo process.

One issue that resulted from this fix was how to reset the c.created
channel during site rebuilds. In the current code, this is done by
assigning c.commandeerHugoState to a newCommandeerHugoState in
*commandeer.fullRebuild. However, this creates a race condition, since
other goroutines can be reading the value of c.created just as
*commandeer.fullRebuild is attempting to reassign it.

This change fixes the race condition by adding the lazy.Notifier type,
which preserves the use of c.created while allowing for it to be reset
in a goroutine-safe way. c.created is now a lazy.Notifier. I added this
type to the lazy package because it didn't seem worth it to add a new
package, but we can do this if it's cleaner. (I was thinking about using
sync.Cond for this, but the approach I used is closer to the original
use of the c.created channel.)

Also adds an integration test for interrupting the server after fixing
an invalid configuration. An earlier version of this change caused hugo
server to stop exiting with a SIGINT after fixing a bad config, so I
wanted to ensure that we can prevent this.

Fixes gohugoio#8340
ptgott added a commit to ptgott/hugo that referenced this issue Mar 17, 2022
If a user starts hugo server and writes to the config file in a way
that renders it invalid, hugo server stops responding to SIGINTs,
meaning that a user needs to send a SIGKILL to the hugo process to stop
it.

*commandeer.serve needs to obtain a *hugolib.HugoSites in order to
close the Hugo environment. After handling signals/the stop channel,
*commandeer.serve calls c.hugo() to get the *hugolib.HugoSites. This
function receives from the c.created channel, which is closed when the
HugoSites is ready. However, if an error took place while loading the
config, c.created would never be closed, so Hugo would block on this
channel even after handling SIGINT.

The solution is to close c.created if we failed to load a config when
rebuilding the site, then check at the end of *commandeer.serve whether
the *hugolib.HugoSites is nil. If it is, we stop waiting for a HugoSites
to close, and exit the hugo process.

One issue that resulted from this fix was how to reset the c.created
channel during site rebuilds. In the current code, this is done by
assigning c.commandeerHugoState to a newCommandeerHugoState in
*commandeer.fullRebuild. However, this creates a race condition, since
other goroutines can be reading the value of c.created just as
*commandeer.fullRebuild is attempting to reassign it.

This change fixes the race condition by adding the lazy.Notifier type,
which preserves the use of c.created while allowing for it to be reset
in a goroutine-safe way. c.created is now a lazy.Notifier. I added this
type to the lazy package because it didn't seem worth it to add a new
package, but we can do this if it's cleaner. (I was thinking about using
sync.Cond for this, but the approach I used is closer to the original
use of the c.created channel.)

Also adds an integration test for interrupting the server after fixing
an invalid configuration. An earlier version of this change caused hugo
server to stop exiting with a SIGINT after fixing a bad config, so I
wanted to ensure that we can prevent this.

Fixes gohugoio#8340
ptgott added a commit to ptgott/hugo that referenced this issue Mar 18, 2022
If a user starts hugo server and writes to the config file in a way
that renders it invalid, hugo server stops responding to SIGINTs,
meaning that a user needs to send a SIGKILL to the hugo process to stop
it.

*commandeer.serve needs to obtain a *hugolib.HugoSites in order to
close the Hugo environment. After handling signals/the stop channel,
*commandeer.serve calls c.hugo() to get the *hugolib.HugoSites. This
function receives from the c.created channel, which is closed when the
HugoSites is ready. However, if an error took place while loading the
config, c.created would never be closed, so Hugo would block on this
channel even after handling SIGINT.

The solution is to close c.created if we failed to load a config when
rebuilding the site, then check at the end of *commandeer.serve whether
the *hugolib.HugoSites is nil. If it is, we stop waiting for a HugoSites
to close, and exit the hugo process.

One issue that resulted from this fix was how to reset the c.created
channel during site rebuilds. In the current code, this is done by
assigning c.commandeerHugoState to a newCommandeerHugoState in
*commandeer.fullRebuild. However, this creates a race condition, since
other goroutines can be reading the value of c.created just as
*commandeer.fullRebuild is attempting to reassign it.

This change fixes the race condition by adding the lazy.Notifier type,
which preserves the use of c.created while allowing for it to be reset
in a goroutine-safe way. c.created is now a lazy.Notifier. I added this
type to the lazy package because it didn't seem worth it to add a new
package, but we can do this if it's cleaner. (I was thinking about using
sync.Cond for this, but the approach I used is closer to the original
use of the c.created channel.)

Also adds an integration test for interrupting the server after fixing
an invalid configuration. An earlier version of this change caused hugo
server to stop exiting with a SIGINT after fixing a bad config, so I
wanted to ensure that we can prevent this.

Fixes gohugoio#8340
ptgott added a commit to ptgott/hugo that referenced this issue Mar 20, 2022
If a user starts hugo server and writes to the config file in a way
that renders it invalid, hugo server stops responding to SIGINTs,
meaning that a user needs to send a SIGKILL to the hugo process to stop
it.

*commandeer.serve needs to obtain a *hugolib.HugoSites in order to
close the Hugo environment. After handling signals/the stop channel,
*commandeer.serve calls c.hugo() to get the *hugolib.HugoSites. This
function receives from the c.created channel, which is closed when the
HugoSites is ready. However, if an error took place while loading the
config, c.created would never be closed, so Hugo would block on this
channel even after handling SIGINT.

The solution is to close c.created if we failed to load a config when
rebuilding the site, then check at the end of *commandeer.serve whether
the *hugolib.HugoSites is nil. If it is, we stop waiting for a HugoSites
to close, and exit the hugo process.

One issue that resulted from this fix was how to reset the c.created
channel during site rebuilds. In the current code, this is done by
assigning c.commandeerHugoState to a newCommandeerHugoState in
*commandeer.fullRebuild. However, this creates a race condition, since
other goroutines can be reading the value of c.created just as
*commandeer.fullRebuild is attempting to reassign it.

This change fixes the race condition by adding the lazy.Notifier type,
which preserves the use of c.created while allowing for it to be reset
in a goroutine-safe way. c.created is now a lazy.Notifier. I added this
type to the lazy package because it didn't seem worth it to add a new
package, but we can do this if it's cleaner. (I was thinking about using
sync.Cond for this, but the approach I used is closer to the original
use of the c.created channel.)

Also adds an integration test for interrupting the server after fixing
an invalid configuration. An earlier version of this change caused hugo
server to stop exiting with a SIGINT after fixing a bad config, so I
wanted to ensure that we can prevent this.

Fixes gohugoio#8340
ptgott added a commit to ptgott/hugo that referenced this issue Mar 26, 2022
If a user starts hugo server and writes to the config file in a way
that renders it invalid, hugo server stops responding to SIGINTs,
meaning that a user needs to send a SIGKILL to the hugo process to stop
it.

*commandeer.serve needs to obtain a *hugolib.HugoSites in order to
close the Hugo environment. After handling signals/the stop channel,
*commandeer.serve calls c.hugo() to get the *hugolib.HugoSites. This
function receives from the c.created channel, which is closed when the
HugoSites is ready. However, if an error took place while loading the
config, c.created would never be closed, so Hugo would block on this
channel even after handling SIGINT.

The solution is to close c.created if we failed to load a config when
rebuilding the site, then check at the end of *commandeer.serve whether
the *hugolib.HugoSites is nil. If it is, we stop waiting for a HugoSites
to close, and exit the hugo process.

One issue that resulted from this fix was how to reset the c.created
channel during site rebuilds. In the current code, this is done by
assigning c.commandeerHugoState to a newCommandeerHugoState in
*commandeer.fullRebuild. However, this creates a race condition, since
other goroutines can be reading the value of c.created just as
*commandeer.fullRebuild is attempting to reassign it.

This change fixes the race condition by adding the lazy.Notifier type,
which preserves the use of c.created while allowing for it to be reset
in a goroutine-safe way. c.created is now a lazy.Notifier. I added this
type to the lazy package because it didn't seem worth it to add a new
package, but we can do this if it's cleaner. (I was thinking about using
sync.Cond for this, but the approach I used is closer to the original
use of the c.created channel.)

Also adds an integration test for interrupting the server after fixing
an invalid configuration. An earlier version of this change caused hugo
server to stop exiting with a SIGINT after fixing a bad config, so I
wanted to ensure that we can prevent this.

Fixes gohugoio#8340
ptgott added a commit to ptgott/hugo that referenced this issue Apr 10, 2022
If a user starts hugo server and writes to the config file in a way
that renders it invalid, hugo server stops responding to SIGINTs,
meaning that a user needs to send a SIGKILL to the hugo process to stop
it.

*commandeer.serve needs to obtain a *hugolib.HugoSites in order to
close the Hugo environment. After handling signals/the stop channel,
*commandeer.serve calls c.hugo() to get the *hugolib.HugoSites. This
function receives from the c.created channel, which is closed when the
HugoSites is ready. However, if an error took place while loading the
config, c.created would never be closed, so Hugo would block on this
channel even after handling SIGINT.

The solution is to close c.created if we failed to load a config when
rebuilding the site, then check at the end of *commandeer.serve whether
the *hugolib.HugoSites is nil. If it is, we stop waiting for a HugoSites
to close, and exit the hugo process.

One issue that resulted from this fix was how to reset the c.created
channel during site rebuilds. In the current code, this is done by
assigning c.commandeerHugoState to a newCommandeerHugoState in
*commandeer.fullRebuild. However, this creates a race condition, since
other goroutines can be reading the value of c.created just as
*commandeer.fullRebuild is attempting to reassign it.

This change fixes the race condition by adding the lazy.Notifier type,
which preserves the use of c.created while allowing for it to be reset
in a goroutine-safe way. c.created is now a lazy.Notifier. I added this
type to the lazy package because it didn't seem worth it to add a new
package, but we can do this if it's cleaner. (I was thinking about using
sync.Cond for this, but the approach I used is closer to the original
use of the c.created channel.)

Also adds an integration test for interrupting the server after fixing
an invalid configuration. An earlier version of this change caused hugo
server to stop exiting with a SIGINT after fixing a bad config, so I
wanted to ensure that we can prevent this.

Fixes gohugoio#8340
ptgott added a commit to ptgott/hugo that referenced this issue Apr 23, 2022
If a user starts hugo server and writes to the config file in a way
that renders it invalid, hugo server stops responding to SIGINTs,
meaning that a user needs to send a SIGKILL to the hugo process to stop
it.

*commandeer.serve needs to obtain a *hugolib.HugoSites in order to
close the Hugo environment. After handling signals/the stop channel,
*commandeer.serve calls c.hugo() to get the *hugolib.HugoSites. This
function receives from the c.created channel, which is closed when the
HugoSites is ready. However, if an error took place while loading the
config, c.created would never be closed, so Hugo would block on this
channel even after handling SIGINT.

The solution is to close c.created if we failed to load a config when
rebuilding the site, then check at the end of *commandeer.serve whether
the *hugolib.HugoSites is nil. If it is, we stop waiting for a HugoSites
to close, and exit the hugo process.

One issue that resulted from this fix was how to reset the c.created
channel during site rebuilds. In the current code, this is done by
assigning c.commandeerHugoState to a newCommandeerHugoState in
*commandeer.fullRebuild. However, this creates a race condition, since
other goroutines can be reading the value of c.created just as
*commandeer.fullRebuild is attempting to reassign it.

This change fixes the race condition by adding the lazy.Notifier type,
which preserves the use of c.created while allowing for it to be reset
in a goroutine-safe way. c.created is now a lazy.Notifier. I added this
type to the lazy package because it didn't seem worth it to add a new
package, but we can do this if it's cleaner. (I was thinking about using
sync.Cond for this, but the approach I used is closer to the original
use of the c.created channel.)

Also adds an integration test for interrupting the server after fixing
an invalid configuration. An earlier version of this change caused hugo
server to stop exiting with a SIGINT after fixing a bad config, so I
wanted to ensure that we can prevent this.

Fixes gohugoio#8340
ptgott added a commit to ptgott/hugo that referenced this issue May 14, 2022
If a user starts hugo server and writes to the config file in a way
that renders it invalid, hugo server stops responding to SIGINTs,
meaning that a user needs to send a SIGKILL to the hugo process to stop
it.

*commandeer.serve needs to obtain a *hugolib.HugoSites in order to
close the Hugo environment. After handling signals/the stop channel,
*commandeer.serve calls c.hugo() to get the *hugolib.HugoSites. This
function receives from the c.created channel, which is closed when the
HugoSites is ready. However, if an error took place while loading the
config, c.created would never be closed, so Hugo would block on this
channel even after handling SIGINT.

The solution is to close c.created if we failed to load a config when
rebuilding the site, then check at the end of *commandeer.serve whether
the *hugolib.HugoSites is nil. If it is, we stop waiting for a HugoSites
to close, and exit the hugo process.

One issue that resulted from this fix was how to reset the c.created
channel during site rebuilds. In the current code, this is done by
assigning c.commandeerHugoState to a newCommandeerHugoState in
*commandeer.fullRebuild. However, this creates a race condition, since
other goroutines can be reading the value of c.created just as
*commandeer.fullRebuild is attempting to reassign it.

This change fixes the race condition by adding the lazy.Notifier type,
which preserves the use of c.created while allowing for it to be reset
in a goroutine-safe way. c.created is now a lazy.Notifier. I added this
type to the lazy package because it didn't seem worth it to add a new
package, but we can do this if it's cleaner. (I was thinking about using
sync.Cond for this, but the approach I used is closer to the original
use of the c.created channel.)

Also adds an integration test for interrupting the server after fixing
an invalid configuration. An earlier version of this change caused hugo
server to stop exiting with a SIGINT after fixing a bad config, so I
wanted to ensure that we can prevent this.

Fixes gohugoio#8340
@jmooring
Copy link
Member

jmooring commented Nov 7, 2022

Unable to reproduce with v0.105.0. Fixed in v0.99.0 with #9899.

@jmooring jmooring closed this as completed Nov 7, 2022
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants