gunicorn icon indicating copy to clipboard operation
gunicorn copied to clipboard

Fix auto reloader

Open bigsbug opened this issue 3 years ago • 6 comments

Auto-reloading with the Uvicron worker after detecting new changes produces the following issues: Fixes #2339 Fixes #2814

Also, I guess there are some issues with the auto-reload, such as: #1562

There is a problem with using the sys.exit() in a forked process (child processes or workers) instead of using the os._exit() that is mentioned in python documentation.

This modification solved the problem and passed all tests.

bigsbug avatar Jun 29 '22 17:06 bigsbug

Hi, @brosner do you can review this PR and merge it?

bigsbug avatar Jun 30 '22 20:06 bigsbug

Changes looks good. I will check though since sys.exit has not been an issue until recently. I would rather prefer that python fix this. Has an issue been open upstream?

benoitc avatar Jun 30 '22 21:06 benoitc

yes there has an open issue for it

bigsbug avatar Jul 01 '22 09:07 bigsbug

Hi, @brosner, I checked this problem with different versions of python and the result are the same !!. and I also checked the documentation of several versions of python and all of them mentioned this situation. I don't think it is an issue with python, and in the python upstream I can't find any related issue. so what do you think?

bigsbug avatar Jul 02 '22 21:07 bigsbug

Hi @brosner no risk merging this right ?

lb-ronyeh avatar May 04 '23 12:05 lb-ronyeh

I don't think I understand this PR. From what I read, it's frequently the case that a program may not want to close flush stdio or call exit handler from a child process, but does Gunicorn? I don't think we register any atexit handlers, but developers might do so in server hooks. Would they expect those to be called when a worker exits? I don't know. This would definitely be a change in behavior.

Do we have an explanation for why it solves the linked issues? I think that's what I'm missing. I'm not understanding the value, only the risk.

tilgovi avatar Dec 27 '23 02:12 tilgovi