Fixes #30420 -- Do not reload runerserver during debugging / request processing

[rohithpr]

Problem

https://code.djangoproject.com/ticket/30420

Saving a file while a debugger is active or when a long running request is being processed will trigger the auto reloader and the previous request's thread will be killed.

Solution

Track the requests started and finished. If the auto reloader has to be invoked, wait till all pending requests have finished before proceeding.

Manual testing proof

• I've added a print statement in wait_for_requests_to_finish to demonstrate that the reloader is waiting for started and finished counts to match.
• The reloader kicks in immediately after continuing from the breakpoint.

http://recordit.co/T0bCnzO2JR

[orf]

Hey 👋

This is a fantastic idea, I really like it. However I have a few thoughts about the implementation.

Firstly we should definitely move this to the BaseReloader class, I think any implementation is generic enough and will work with any current (or future) reloader. We just want to detect if there is a currently executing request, and if so set a flag and have the reload trigger once the request is finished.

But this is trickier than it sounds. While I quite like the elegant approach you've taken with a counter tied to the request signals I feel like this might be a bit too fragile. There's a risk with any additional features like this that makes me a bit nervous: the development experience is greatly impacted if the autoreloader does not function correctly. The failure mode here is if the request_finished signal is not correctly dispatched, and if this ever happens for any reason then the autoreloader will fail outright. There is historical precedent for this and while I'm not aware of anything else like this it does feel like it might crop up somehow during development - perhaps in ways that have so far gone unnoticed.

It's also arguable that people want long-running requests to be interrupted by the autoreloader - perhaps your current code is in an infinite loop or is doing something else that it shouldn't. Note that the linked Flask PR (https://github.com/pallets/werkzeug/pull/1525) was closed for similar reasons.

So I think we should focus on the "while debugging" aspect rather than the "while a request is processing" aspect. With the introduction of the sys.breakpointhook function in PEP 553 and the breakpoint audit hook it would be awesome if we could somehow detect if a debugger is currently active and use that as a condition? If we can detect that a thread is being debugged for the popular debuggers (ipdb, pydev and sys.breakpointhook) then the implementation might be a lot more resilient.

[rohithpr]

Hi @orf! Pinging you to see if you had a chance to check out my comments above. Please let me know if you have any thoughts about that, or if this PR can proceed with the current implementation.

[nessita]

Hello @rohithpr! I'm doing some PR cleanup and I was wondering if you would have time to move this forward. The PR could benefit from an update (rebase onto main and fix conflicts), and then perhaps we could re-ping @orf to continue with their review. What do you think? Thanks, Natalia.

[shangxiao]

Just saw this from Nessita's bump. I would absolutely love this feature ❤️ Let us know if any assistance required.

[rohithpr]

Thanks for the ping, @nessita. I've rebased the branch.

Hi, @orf! Would you be able to review my comments above? I would like to confirm that this approach is acceptable before spending time fixing tests etc.

[nessita]

Just saw this from Nessita's bump. I would absolutely love this feature heart Let us know if any assistance required.

@shangxiao Would you be willing/able to review the PR? I'm not sure if @orf would be available to continue with the review.

[orf]

Hey @nessita, sorry for the late reply. I’ve been busy the last few weeks, but I’ll take a look on Monday!

[nessita]

Hey @nessita, sorry for the late reply. I’ve been busy the last few weeks, but I’ll take a look on Monday!

Amazing, thank you!

[nessita]

Hey @nessita, sorry for the late reply. I’ve been busy the last few weeks, but I’ll take a look on Monday!

Friendly ping! If you can't continue with the review, we'll try to seek other reviewers. Let us know!

[felixxm]

@rohithpr Thanks for your investigation :+1: Unfortunately :disappointed: , even if currently it's not possible to control when users debug, that doesn't mean that "request" approach proposed in this PR is acceptable. I have to agree with Tom that it's too fragile, and in some cases may not be what developers want.