ansys / ansys/pydynamicreporting
Make TemplateEngine shared render state exception-safe and thread-safe
- Dominant language
- Python
- Stars
- 12
- Forks
- 5
- Avg merge
- 3d 14h
- Merged PRs (30d)
- 9
Description
## Summary
Serverless report rendering writes process-wide `TemplateEngine` class state. The render path resets print style in `finally`, but it does not restore every other shared value after success or failure. A failed render can therefore leave state from report A available to report B. Concurrent renders can also interleave writes unless every caller adds its own process-wide lock.
Affected source at commit `7d4dcf4cbfc3652f28cb9d8f053d05d1d237d038`:
https://github.com/ansys/pydynamicreporting/blob/7d4dcf4cbfc3652f28cb9d8f053d05d1d237d038/src/ansys/dynamicreporting/core/serverless/template.py#L617-L644
## Current call path
`ADR.render_report()` calls `Template.get(...).render(...)` synchronously. `Template.render()` then does the following:
1. Calls `TemplateEngine.set_print_style(...)`.
2. Calls `TemplateEngine.set_global_context(...)` with the current root template.
3. Calls `TemplateEngine.start_toc_session()`.
4. Calls `engine.render(...)`.
5. Calls `TemplateEngine.end_toc_session()` only when step 4 completes.
6. Resets print style to `None` in `finally`.
7. Catches rendering exceptions and returns product error HTML.
The print style has explicit cleanup. The global render context and table-of-contents session do not have equivalent guaranteed cleanup in this path. In particular, an exception from `engine.render(...)` skips `end_toc_session()`.
Because the outer exception handler converts the failure into error HTML, downstream callers cannot reliably detect the internal failure and repair `TemplateEngine` state themselves.
## Why this matters
`TemplateEngine` stores render data in class variables shared by every render in the Python process. This creates two separate problems:
- Failure recovery: report A can fail after changing shared state, and report B can start with values left by report A.
- Concurrency: report A and report B can write the same class variables at the same time when rendered from separate threads.
The serverless API documents one ADR setup shared by threads. Shared render state therefore needs product-owned isolation or serialization; each caller should not have to know private `TemplateEngine` fields and reset rules.
## Deterministic failure reproduction
Extend the existing `test_template_render_resets_print_style_after_render` coverage with a fake or instrumented `TemplateEngine` that records all shared state.
1. Give report A a fake engine whose `render(...)` method raises after `start_toc_session()`.
2. Call `report_a.render(context={"print": "pdf"})`.
3. Confirm that product error HTML is returned.
4. Inspect every `TemplateEngine` class variable changed by the call.
5. Assert that each variable equals its value from before step 2.
6. Start report B and assert that no context, root template, page state, or TOC data from report A is visible.
The current implementation resets print style but does not satisfy steps 5 and 6 for all shared render state.
## Deterministic concurrency reproduction
Use two threads and events rather than sleeps.
1. Thread A enters rendering after writing its root and TOC state, then waits on an event inside its fake engine.
2. Thread B starts rendering a different report and reaches the same shared-state section.
3. Record the root, page, print, and TOC state seen by both engines.
4. Release thread A and join both threads.
5. Assert that each output contains only its own report state.
Without product-owned serialization or render-local state, the class-variable writes can interleave.
## Expected behavior
- A render restores every shared `TemplateEngine` value to its pre-call state on success and on every exception path.
- A failed TOC or engine render cannot leave an open or stale TOC session.
- Two supported concurrent renders cannot observe or overwrite each other state.
- HTML render, HTML export, browser-PDF export, and any other path using the same engine follow the same rule.
## Actual behavior
- Print style is reset in `finally`.
- Other shared render values are not all restored.
- `end_toc_session()` is skipped when `engine.render(...)` raises.
- Callers must serialize render and export operations themselves to prevent overlap.
- Caller-side serialization does not repair state left after a failed render.
## Proposed fix
Preferred fix:
- Move mutable render state from `TemplateEngine` class variables into state owned by one render invocation. A render context object or `contextvars` can preserve nested call behavior without sharing values across worker threads.
Minimum safe fix if render-local state is not practical now:
1. Add one product-owned process-wide render lock used by every render and export entry point.
2. Snapshot every shared `TemplateEngine` value before changing it.
3. Put all restoration in one `finally` block.
4. Add an explicit TOC abort or reset operation that is safe when a session started but did not finish.
5. Restore the previous values, not assumed defaults, so nested or embedded rendering remains correct.
6. Preserve the original render exception and existing error-HTML behavior.
A product-owned fix is necessary because PyDR knows the complete class-state list and valid reset behavior. Downstream projects should not import private engine modules or duplicate that knowledge.
## Acceptance checks
- A normal render restores all preexisting shared state.
- An exception from `engine.render(...)` restores all preexisting shared state.
- An exception from TOC finalization restores all preexisting shared state.
- A render that returns product error HTML still leaves clean state.
- Sequential report B output contains no state from failed report A.
- Two threaded renders are isolated or deliberately serialized by PyDR.
- HTML render, HTML export, and browser-PDF export use the same protection.
- Existing render output and print-style behavior remain unchanged.
## Downstream workaround
`pydynamicreporting-mcp` commit `d847be3399f09df7d936b3aa584c6dd05f36a406` uses one module-level `threading.Lock` around complete PyDR render and export calls. This prevents overlapping MCP operations across backend instances in one process. It deliberately does not access private `TemplateEngine` state and cannot clean state left by a failed product render.
Contributor guide
Assessment
This issue has not been assessed yet.