[PATCH] auto-reload fail by design

[anatoly techtonik <techtonik@…>]

It is rather funny that nobody spotted it before, but.. autoreload option designed for development mode actually stops to work once you have a syntax error in your script and the script fails to load.

branches/0.11-stable/trac/util/autoreload.py@7889​

How does this happen. The normal flow with --auto-reload is to start new process for Trac and keep current process solely for monitoring how child behaves. If the child exits with the error code 3, it is restarted. Child itself runs two threads - first it creates thread where a main function is executed. This function processes requests and as a side effect imports all required files filling sys.modules list (shared with main thread, because, well, everything is shared between threads). Main thread then goes into infinite loop to periodically check timestamps of files in sys.modules and exit with error code 3 if timestamp mismatches.

Now the problem. If there is a syntax error in .py file it is not added into sys.modules and will not be checked to restart child process when the error is fixed.

P.S. I've added it to 0.11.7, because I'd like to see any fix in 0.11. It will be handy for exploring plugins behavior when porting them to 0.12

[cboos]

Well, as we know this since ages, we took the habit of not making syntax errors ;-)

Seriously, for the few occasions this happens, we just have to restart tracd (make server), at the appropriate time, i.e. after fixing the error.

If you have an idea about how to make Trac notice this by itself(?), then please provide us with a patch, otherwise this will likely become a wontfix.

Also, in general, there won't be any more change to 0.11-stable, unless someone takes over maintenance (osimons?).

[anatoly techtonik <techtonik@…>]

An idea. When there is a Trac[loader] ERROR: Failed to load plugin from XXX - add this XXX to a list of extra files to be monitored.

What is the proper way to pass the list of extra files from main function to monitoring thread? I can try to create a patch that uses global variable, but they say global variables are evil.

[anatoly techtonik <techtonik@…>]

Attaching patch for 0.11.7. Code Review link for those who knows ​http://codereview.appspot.com/2131042/

[anatoly techtonik <techtonik@…>]

autoreload fix

[cboos]

Better add to a set than repeatedly append to a list...

And don't you want to restrict that to SyntaxError exceptions?

[anatoly techtonik <techtonik@…>]

Replying to cboos:

Better add to a set than repeatedly append to a list...

set was added in Python 2.4 and I believe Trac 0.11 is still Python 2.3 compatible. This should not be a problem as the loader fails only once for every imported file.

And don't you want to restrict that to SyntaxError exceptions?

I am not sure that all errors that cause import failure are strictly syntax-related. For example, I get "Failed to load plugin" entry in trac.log for ZeroDivisionError?.

[anatoly techtonik <techtonik@…>]

do not add files several times

[anatoly techtonik <techtonik@…>]

Added check for file presence. Manually checked that ZeroDivisionError? stops autoreloading mechanizm. I assume that any unhandled run-time exception during the import (and hence execution) of module will do this.

[anonymous]

+1 for including this in the next release

[cboos]

?

[anatoly techtonik <techtonik@…>]

Maybe we should make issue "stars" more visible?

[cboos]

Feels a bit hackish, but I'll give the patch a try.

[anatoly techtonik <techtonik@…>]

There might be one more case not covered by this patch when autorefresh stops to work. When some .py file from plugins/ directory is removed (not necessary a plugin). This usually happens with test111.py scripts created to pinpoint some problem.

My high level assumption is that auto-reload fails, because of stacktrace or some exit condition in monitor thread. Perhaps intercepting this condition with obligatory output into trac.log will help to make less hackish patch.