Fix/routes reload crash

[Abdallah-A-Abdelhalem]

Fix Routes server killing Revit on reload

Description

Reloading pyRevit while the Routes server is running leaves the server dead, and
the next request terminates the Revit process.

Three defects in pyrevitlib/pyrevit/routes/server/server.py combine to produce
this. They share one hazard: requests are served on threads that are never STA,
while pyRevit directs stderr to its script output console — a WPF window that
can only be created on Revit's STA UI thread. Anything printed from a serving
thread therefore tries to build a WPF window off the UI thread and raises
The calling thread must be STA. Unhandled on a thread, that terminates the
process.

Revit vanishes with no dialog, leaving only an Application event log entry
(.NET Runtime, ID 1026):

System.InvalidOperationException: The calling thread must be STA,
because many UI components require this.
   at System.Windows.Window..ctor()
   at pyRevitLabs.MahAppsMetro.Controls.MetroWindow..ctor()
   at PyRevitLabs.PyRevit.Runtime.ScriptConsoleTemplate..ctor()
   at PyRevitLabs.PyRevit.Runtime.ScriptConsole..ctor(...)
   at PyRevitLabs.PyRevit.Runtime.ScriptIO.GetOutput()
   at PyRevitLabs.PyRevit.Runtime.ScriptIO.Write(...)
   at IronPython.Runtime.Operations.PythonOps.PrintWithDest(...)
   at IronPython.Modules.PythonThread.ThreadObj.Start()

That stack describes the error reporter, not the error. PrintWithDest
invoked directly from ThreadObj.Start(), with no user frame between, means an
exception escaped a serving thread and IronPython was printing it — and the
printing is what kills the process.

Reproduction

Verified on Revit 2026 against an unmodified develop build:

1. Start Revit. No model, no pyRevit tool — nothing but the next step.
2. pyRevit → Reload
3. Issue any Routes request

The request returns nothing and Revit terminates with the event above. Measured
after step 2 on develop: zero listeners on the Routes port. Opening a model
or running a pyRevit tool first is not required; both were tested and made no
difference.

The three defects

Each is a separate commit and can be reviewed independently. The measurements
referenced here are in the Testing section.

1. Request logging writes to stderr

BaseHTTPRequestHandler.log_message() writes an access-log line to sys.stderr
for every request, reached from send_response() via log_request(). That is
the hazard above, on the response path of every request.

Request logging now goes through mlogger, whose _emit already suppresses
failures from the logging service and is therefore safe to call off the UI
thread. log_error() and log_request() both reach stderr through
log_message(), so overriding that one method covers all of it.

Measured: with defects 2 and 3 fixed but this one left in place, a request
after a reload returns no HTTP response at all — the connection is accepted
and closed in ~30 ms with an empty body, because the exception is raised part
way through writing the response. Adding this fix, and changing nothing else,
returns a correct 503 with a JSON body. Runs 5 and 6 below differ by this
commit alone.

2. Exceptions escape serving threads

ThreadingMixIn.process_request_thread guards the request itself but not the
per-request cleanup that follows. Stopping the server closes sockets while
requests are in flight, so that cleanup raises on an already closed socket and
the exception escapes the thread without ever reaching handle_error. The
whole request path is now guarded, and handle_error no longer prints a
traceback to stderr.

Measured: this is the defect the reproduction above triggers. Both captured
crashes show PrintWithDest called directly from ThreadObj.Start(), i.e. an
exception escaping a thread rather than the log_message path. With this fix
applied, the same sequence leaves Revit running.

3. Two accept loops per server

RoutesServer.__init__ calls self.start(), and activate_server() calls
start() again on the instance it just constructed. Two serve_forever loops
run against one socket while only the second is tracked, so the first is never
joined on shutdown. start() is now a no-op when the loop is already running.

Measured: after a reload, develop leaves zero listeners on the Routes
port — the server does not come back. This branch leaves one, serving
normally, across three consecutive reloads. The double call itself is plain in
the source. The listener measurement is for the branch as a whole; this commit
was not tested in isolation.

---

Checklist

• Code follows the PEP 8 style guide.
• Code has been formatted with Black —
  black --check reports the file unchanged.
• Changes are tested and verified to work as expected. See below.

ruff check reports 12 violations after this change versus 13 before —
all pre-existing D102s on untouched methods; one is resolved because start()
gained a docstring. No new violations.

All added code is IronPython 2.7 compatible: no f-strings, annotations, or
py3-only syntax. Thread.is_alive() is available in 2.6+.

---

Testing

Revit 2026. Every run used the same sequence — launch, reload pyRevit, issue a
request — varying only the contents of server.py. A client extension that
patches these same methods at runtime was disabled throughout so it could not
mask results. No model was open unless stated.

Run	Build	Listeners after reload	Request	Revit
1	develop	n/a (no reload)	503, correct body	alive
3	develop	0	no response	terminated
5	defects 2+3	1	HTTP 000, empty body	alive
6	defects 1+2+3	1	503, correct body	alive

• Runs 1 and 3 isolate the reload as the trigger.
• Runs 3 and 5 isolate defect 2: the process survives.
• Runs 5 and 6 isolate defect 1: the response is actually delivered.

A 503 is the correct response when no model is open, so it indicates a fully
served request.

On this branch, across three consecutive reloads with a continuous request
stream running, the endpoint returned to service every time and Revit stayed
responsive. No .NET Runtime 1026 events were recorded during any run carrying
this branch.

With a model loaded on this branch, every route exposed by the client returned
200, exercising both the GET and POST paths and a binary response:

Route	Status	Response
/status/	200	150 B
/model_info/	200	5.7 KB
/list_levels/	200	167 B
/list_views/	200	542 B
/current_view_info/	200	246 B
/list_family_categories/	200	937 B
/list_families/ (POST)	200	682 B
/current_view_elements/ (POST)	200	1.2 KB
/get_view/	200	56 KB PNG

Requests were issued sequentially; RequestHandler holds request state in
module-level singletons, so concurrent requests overwrite one another
independently of this change.

---

Additional Notes

• Production change is minimal: 88 insertions in server.py, entirely additive or defensive, no existing behaviour removed. A further 290 lines are unit tests.
• Two approaches were tried and rejected. Reordering shutdown() to stop the
  loop before closing the socket makes HTTPServer.shutdown() block the UI
  thread waiting for an acknowledgement, so the reload hangs instead of
  crashing. Calling server_close() during teardown closes a socket that may
  still be serving a request, and terminated Revit when tried.

[devloai]

PR Summary:

• Fixes a crash where reloading pyRevit while the Routes server is running kills Revit on the next request.
• Three defects in server.py are addressed: (1) log_message now routes through mlogger instead of stderr (WPF console creation off the STA thread), (2) process_request_thread fully guards finish_request + shutdown_request so no exception escapes a serving thread, (3) start() is a no-op when the accept loop is already running, preventing a double serve_forever from __init__ + activate_server().
• All changes are additive/defensive, IronPython 2.7 compatible, and verified against Revit 2026.

Review Summary:

The three fixes are well-isolated, correctly target the identified defects, and the reasoning is sound — the log_message override covers all access logging, the process_request_thread guard closes the unguarded cleanup path, and the start() no-op prevents the leaked second accept loop. The only issues found are two except Exception: pass blocks in process_request_thread that silently swallow exceptions without logging, which conflicts with the repo's coding guideline that catch blocks must log at minimum. Both are quick one-line mlogger.debug() additions that preserve the defensive intent while leaving a diagnostic trail. Knowledge utilized: repo coding guidelines (empty catch block logging requirement), IronPython 2.7 compatibility checklist, and the exception/logging visibility review checklist.

Suggestions

• Add unit tests for process_request_thread guarding shutdown_request on a closed socket Apply
• Investigate the leaked second server bound to the port after repeated reloads mentioned in the third commit message Apply

[devloai]

Added unit tests for the request-path guard in ThreadedHttpServer — pyrevitlib/pyrevit/unittests/test_routes_server_threading.py (13 tests, +290 lines, no production code touched).

ProcessRequestThreadTests

• successful request is served and its socket cleaned up
• cleanup raising on a closed socket (socket.error, ValueError, RuntimeError) does not escape, and cleanup is still attempted
• a cleanup failure is not reported as a request failure (handle_error untouched)
• the inherited cleanup path runs against a genuinely closed socket.socket() without escaping
• a request failure is routed to handle_error and cleanup still runs
• request + cleanup failing together are both contained
• a failure inside handle_error itself does not escape
• nothing on the failing path writes to sys.stderr (the STA hazard)
• the guard holds when the method runs on a real threading.Thread — the thread finishes and records no escaped exception

HandleErrorTests — errors are reported through mlogger (client address present in the message) and never reach sys.stderr.

Verification:

• All 13 pass against this branch.
• Mutation check: with ThreadedHttpServer.process_request_thread removed so the inherited ThreadingMixIn implementation is used, 7 of the 13 fail — including the thread test, which reports [OSError('closed socket')] escaping. The tests fail without the fix, so they are actually pinning the behaviour.
• ruff check clean, black clean.
• IronPython 2.7 compatible: no f-strings, no unittest.mock, hand-written stubs in the style of the existing test_routes_server_* modules.

The file uses the test_routes_ prefix, so the existing Routes Module Tests button in pyRevitDevTools discovers and runs it automatically — no runner changes needed.

[Copilot]

Pull request overview

This PR fixes a crash where reloading pyRevit while the Routes server is running would kill the Revit process on the next request. The root cause is that requests are served on non-STA threads while pyRevit redirects stderr to a WPF output console that can only be created on Revit's STA UI thread; any exception printed from a serving thread raises The calling thread must be STA and terminates the process. The fix hardens pyrevitlib/pyrevit/routes/server/server.py against three distinct defects and adds unit tests for the threaded request path.

Changes:

• Override HttpRequestHandler.log_message to route request logging through mlogger instead of stderr.
• Override ThreadedHttpServer.handle_error and process_request_thread so no exception (including per-request socket cleanup after shutdown) escapes a serving thread, and nothing reaches stderr.
• Make RoutesServer.start() idempotent so activation no longer spawns a second, untracked serve_forever loop on the same socket; the loop thread is also guarded.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pyrevitlib/pyrevit/routes/server/server.py	Adds defensive overrides (log_message, handle_error, process_request_thread) and an idempotent, guarded start() to keep serving-thread failures off stderr and prevent duplicate accept loops.
pyrevitlib/pyrevit/unittests/test_routes_server_threading.py	New unit tests covering the happy path, contained serving/cleanup/error-reporting failures, stderr isolation, and handle_error logging behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.