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

20.0.27 attach-daemon not responding to touching .ini file #2681

Closed
LiteWait opened this issue Sep 26, 2024 · 12 comments · Fixed by #2689
Closed

20.0.27 attach-daemon not responding to touching .ini file #2681

LiteWait opened this issue Sep 26, 2024 · 12 comments · Fixed by #2689

Comments

@LiteWait
Copy link

LiteWait commented Sep 26, 2024

<= 20.0.26 we've never had an issue with a couple of attach-daemon entries in our .ini files.
Now with 20.0.27 touching the file doesn't restart the daemons. We've changed nothing.

UPDATE: Confirmed, touching an ini that used to restart python app and attach-daemon's no longer works on .27. We downgraded to .26 no problems at all.

Of course, restarting uWSGI works just fine.

@xrmx
Copy link
Collaborator

xrmx commented Sep 27, 2024

@LiteWait any chance you can bisect which commit broke it? Is it 65577d6 ?

@joshzcold
Copy link

joshzcold commented Oct 1, 2024

@xrmx 2.0.27 broke something that seems related to our work flow.

We expect that when we change python modules that workers get reloaded when using --py-autoreload 3

Example of working behavior

[uwsgi-python-reloader] module/file /sm/app/./purchase/service/online_store_service.py has been modified
...gracefully killing workers...
Gracefully killing worker 2 (pid: 423)...
Gracefully killing worker 3 (pid: 424)...
worker 2 buried after 1 seconds
worker 3 buried after 1 seconds

After the update to 2.0.27 we just get

[uwsgi-python-reloader] module/file /sm/app/./purchase/service/online_store_service.py has been modified

But the running application doesn't get the changes to the module.

I can give bisecting and build uwsgiing a shot and see if I can track down the issue.

@joshzcold
Copy link

joshzcold commented Oct 1, 2024

@xrmx I tracked it down to 8f1d0e5

7206318 does not have this issue

My guess is that waitpid waits forever

	for (i = 1; i <= uwsgi.numproc; i++) {
		if (uwsgi.workers[i].pid > 0) {
			waitpid(uwsgi.workers[i].pid, &waitpid_status, 0);
		}
	}

Putting log statements around this confirms that it is waiting forever

	for (i = 1; i <= uwsgi.numproc; i++) {
		if (uwsgi.workers[i].pid > 0) {
			uwsgi_log("Waitng for uwsgi worker pid to shutdown... (pid: %d)", uwsgi.workers[i].pid);
			waitpid(uwsgi.workers[i].pid, &waitpid_status, 0);
			uwsgi_log("Done waiting for uwsgi worker pid to shutdown... (pid: %d)", uwsgi.workers[i].pid);
		}
	}

image

@joshzcold
Copy link

joshzcold commented Oct 1, 2024

In my case at

void uwsgi_detach_daemons() {
in uwsgi_detach_daemons()

struct uwsgi_daemon *ud = uwsgi.daemons; has nothing so the cleanup of "daemon" doesn't happen.

	uwsgi_log("---> ud %s", ud->pid);
	while (ud) {
*** backtrace of 3946844 ***
/home/joshua/uwsgi/uwsgi(uwsgi_backtrace+0x33) [0x64f62f329863]
/home/joshua/uwsgi/uwsgi(uwsgi_segfault+0x27) [0x64f62f329c97]
/usr/lib/x86_64-linux-gnu/libc.so.6(+0x42520) [0x75596a242520]
/home/joshua/uwsgi/uwsgi(uwsgi_detach_daemons+0x41) [0x64f62f3197b1]
/home/joshua/uwsgi/uwsgi(uwsgi_destroy_processes+0x7c) [0x64f62f2ed62c]
/home/joshua/uwsgi/uwsgi(grace_them_all+0x97) [0x64f62f329437]
/usr/lib/x86_64-linux-gnu/libc.so.6(+0x42520) [0x75596a242520]
/usr/lib/x86_64-linux-gnu/libc.so.6(epoll_wait+0x1a) [0x75596a325dea]
/home/joshua/uwsgi/uwsgi(event_queue_wait+0x39) [0x64f62f31bf29]
/home/joshua/uwsgi/uwsgi(master_loop+0xb91) [0x64f62f2ec1c1]
/home/joshua/uwsgi/uwsgi(uwsgi_run+0x2da) [0x64f62f32eaba]
/home/joshua/uwsgi/uwsgi(+0x3eea4) [0x64f62f2d6ea4]
/usr/lib/x86_64-linux-gnu/libc.so.6(+0x29d90) [0x75596a229d90]
/usr/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80) [0x75596a229e40]
/home/joshua/uwsgi/uwsgi(_start+0x25) [0x64f62f2d6ed5]
*** end of backtrace ***

Here is my uwsgi config section if it helps

[uwsgi]
uwsgi-socket = 0.0.0.0:8888
http-socket = 0.0.0.0:8080

buffer-size = 16384

pidfile = ./uwsgi.pid

master = true

chdir = .

home = ./.venv

disable-sendfile = true

need-app = true

logformat = %(addr) - %(user) %(wid) [%(ltime)] "%(method) %(var.PATH_INFO) %(proto)" %(status) %(size) "%(referer)" "%(uagent)"
paste-logger = %p

# tuning configurations
enable-threads = true
vacuum = true
lazy-apps = true

Add my call to uwsgi

~/uwsgi/uwsgi --ini-paste config/.flow-local.ini --die-on-term --disable-sendfile --py-autoreload 1

@joshzcold
Copy link

We could add a kill to the wait loop in

for (i = 1; i <= uwsgi.numproc; i++) {

	for (i = 1; i <= uwsgi.numproc; i++) {
		if (uwsgi.workers[i].pid > 0) {
			kill(uwsgi.workers[i].pid, SIGINT);
			waitpid(uwsgi.workers[i].pid, &waitpid_status, 0);
		}
	}

But ill leave that up to your direction.

@videda
Copy link

videda commented Oct 2, 2024

The SIGHUP kill signal and the uwsgi --reload command described in the docs also do not work in 2.0.27 but work in 2.0.26.

@marceltschoppch
Copy link

The SIGHUP kill signal and the uwsgi --reload command described in the docs also do not work in 2.0.27 but work in 2.0.26.

Can confirm. Any progress on this issue?

Maffooch added a commit to DefectDojo/django-DefectDojo that referenced this issue Oct 9, 2024
Hot reloading appears to be broken in. 2.0.27. The linked GitHub issue is the same behavior that I am seeing

unbit/uwsgi#2681
@prupert
Copy link

prupert commented Oct 10, 2024

I can also confirm that uwsgi reload is broken since 2.0.27. Downgrading to 2.0.26 and awaiting a fix.

@Lalufu
Copy link
Contributor

Lalufu commented Oct 10, 2024

Looking at the code added in 8f1d0e5, I'd move that into gracefully_kill_them_all() instead, before the call to uwsgi_destroy_processes()? That way the processes still get waited for, but it's in that specific path, and not in the general uwsgi_destroy_processes() path which gets called from other contexts as well where the workers haven't been signalled (yet)

Maffooch added a commit to DefectDojo/django-DefectDojo that referenced this issue Oct 11, 2024
Hot reloading appears to be broken in. 2.0.27. The linked GitHub issue is the same behavior that I am seeing

unbit/uwsgi#2681
@Lalufu
Copy link
Contributor

Lalufu commented Oct 15, 2024

I've built packages for Fedora with this patch

https://src.fedoraproject.org/rpms/uwsgi/blob/rawhide/f/uwsgi-2.0.27-graceful-reload.patch

which moves the waitpid into gracefully_kill_them_all(), and as far as I can make out this fixes both the reload-on-touch problem, and also the issue originally reported in #2656
Still waiting on someone besides me confirming that this works.

pedrohdjs pushed a commit to pedrohdjs/django-DefectDojo-sorting that referenced this issue Oct 21, 2024
Hot reloading appears to be broken in. 2.0.27. The linked GitHub issue is the same behavior that I am seeing

unbit/uwsgi#2681
@mjurosz
Copy link

mjurosz commented Oct 22, 2024

@Lalufu Would you also please create Pull Request? I wish @xrmx can review and merge.

@mjurosz
Copy link

mjurosz commented Oct 22, 2024

IMO this is major regression and I would suggest 2.0.27.1 bugfix release to save other users some troubles/time.

Lalufu added a commit to Lalufu/uwsgi that referenced this issue Oct 22, 2024
As part of a fix for unbit#2656, commit 8f1d0e5 introduced a measure to wait
for processes to finish work while reloading. The code to do this
affects other logic flows was well, breaking the ability to reload uwsgi
when a config file changes.

This moves the code around a bit so unbit#2656 stays fixed, but
reload-on-touch/change works again.

Fixes unbit#2681.
@xrmx xrmx closed this as completed in e77e41b Oct 26, 2024
xrmx added a commit to xrmx/uwsgi that referenced this issue Oct 26, 2024
* Fix reload-on-touch/change

As part of a fix for unbit#2656, commit 8f1d0e5 introduced a measure to wait
for processes to finish work while reloading. The code to do this
affects other logic flows was well, breaking the ability to reload uwsgi
when a config file changes.

This moves the code around a bit so unbit#2656 stays fixed, but
reload-on-touch/change works again.

Fixes unbit#2681.

* Apply suggestions from code review

---------

Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants