Skip to content

Conversation

@peter-lawrey
Copy link
Member

@peter-lawrey peter-lawrey commented Nov 21, 2025

This PR improves the Java Runtime Compiler’s cached compiler lifecycle, file I/O robustness, and documentation/agent guidance, while maintaining compatibility with existing callers.

Functional changes

  • Thread-safe compiler and file-manager initialisation

    • CompilerUtils now treats s_compiler and s_standardJavaFileManager as volatile and introduces currentCompiler() plus a synchronised reset():

      • Guarantees a single initialisation path for the underlying JavaCompiler.
      • Clears s_standardJavaFileManager whenever the compiler is reset, preventing a file manager from being tied to a stale compiler instance.
    • CachedCompiler.compileFromJava(...) and loadFromJava(...) now always use currentCompiler():

      • Removes assumptions that s_compiler is already initialised.
      • Ensures repeated calls from multiple threads see a valid compiler and a lazily-initialised StandardJavaFileManager.
    • These changes reduce the risk of races and NullPointerExceptions when the cached compiler is used concurrently or after a reset.

  • Safer file I/O and backup handling

    • CompilerUtils.readBytes(...) now uses Files.newInputStream(file.toPath()) in a try-with-resources block:

      • Preserves existing behaviour (returns null on missing files, throws on I/O errors) but closes streams reliably.
    • CompilerUtils.writeBytes(...):

      • Uses StandardCharsets.UTF_8 for logging decoded content.
      • Only renames existing files to .bak when necessary, and logs when rename/delete/restore operations fail.
      • On write failure, attempts to restore from backup and logs failures explicitly.
    • Net effect: same successful-path behaviour, but more predictable error handling and better diagnostics when I/O fails.

  • Cached compilation behaviour

    • compileFromJava(...) now reuses a lazily-initialised StandardJavaFileManager via a local variable and double-checked initialisation under a CompilerUtils.class lock.
    • The method returns fileManager.getAllBuffers() directly rather than via a temporary local, with no change to the returned data.
    • loadFromJava(...) continues to create MyJavaFileManager instances per class loader, but now uses the currentCompiler() helper for consistency with the rest of the codebase.
  • Dependency alignment

    • Bumped net.openhft:third-party-bom from 3.27ea5 to 3.27ea7 to align this module with the latest shared dependency set used across Chronicle projects.
  • Test harness behaviour

    • FooBarTee now derives the Foo’s integer argument from name.length() instead of a hard-coded literal.
    • Several tests create ClassLoader instances via AccessController.doPrivileged(...), ensuring they continue to work correctly under a security manager / restricted environments.
    • RuntimeCompileTest.outOfBounds adds a stricter type assertion for the thrown IllegalArgumentException, clarifying expected behaviour.

Non-functional changes

  • Language, encoding, and agent guidance (AGENTS.md)

    • Updated language/encoding policy from “ASCII-7 only” to ISO-8859-1 (code points 0–255), still discouraging smart quotes, non-breaking spaces, and accented characters.

    • Added explicit reference to the University of Oxford style guide for British English spelling.

    • Documented practical tools for catching stray non-ASCII characters (e.g. iconv and IDE inspections).

    • Clarified Javadoc and inline comment guidance with concrete “good vs bad” examples, emphasising comments that add behavioural insight instead of restating the obvious.

    • Extended build guidance:

      • Recommend mvn -q clean verify from a clean checkout.
      • Document shared Chronicle quality profiles: -P quality, -P sonar, and module-scoped -P module-quality.
    • Added a “When to open a PR” section to align contributor workflow (green build, focused diffs, linked issues/decisions).

    • Introduced a security checklist for reviewers:

      • Encourages a security pass on every PR (validation, auth, encoding, overflow, resource exhaustion, timing attacks).
      • Reiterates “never commit secrets” and recommends secret-management tooling.
      • Instructs authors to document intentional security trade-offs (e.g. unchecked casts for performance) in Javadoc/AsciiDoc.
  • Requirements and decision-log structure & traceability

    • Moved the project requirements from src/main/adoc to src/main/docs and wired AGENTS.md links to the new locations.

    • project-requirements.adoc:

      • Enabled :sectnums: and added :source-highlighter: rouge.
      • Added an introductory paragraph explaining the JRC Nine-Box-style requirement tags and how they tie into code and decisions.
      • Normalised headings (“Non-Functional - Performance”, etc.) and clarified comparison operators (<= instead of typographic symbols).
      • Added explicit cross-links from requirements to decisions (e.g. JRC-FN-005 → RC-FN-002/RC-RISK-026, JRC-NF-P-006 → RC-NF-P-006, JRC-TEST-014 → RC-TEST-002).
    • decision-log.adoc:

      • Replaced the minimal header with a fully-structured AsciiDoc document (title, :toc:, :sectnums:, :lang: en-GB, :source-highlighter: rouge).

      • Added a Decision Index and expanded the log with new entries:

        • RC-FN-002 – cached compiler API and custom class loaders.
        • RC-NF-P-006 – performance & metaspace budget for cached compilation.
        • RC-RISK-026 – safe handling of debug artefact directories.
      • Existing decisions (RC-FN-001, RC-TEST-002) were updated to reference the relocated requirements paths.

    • Improved decision-record template and AsciiDoc conventions in AGENTS.md:

      • Standardised field labels using Term :: notation (Date, Context, Decision Statement, Alternatives, Rationale, Impact & Consequences, Notes/Links).
      • Clarified list-structuring and indentation rules.
      • Added a “Section Numbering” example showing :sectnums: usage and discouraging manual numeric prefixes in headings.
  • Code quality, style, and logging

    • Switched to StandardCharsets.UTF_8 instead of Charset.forName("UTF-8") and removed legacy UnsupportedEncodingException branches.

    • Tightened logging in MyJavaFileManager to use SLF4J placeholders ([class={}]) rather than string concatenation.

    • Cleaned up minor style issues:

      • Removed redundant modifiers (public int get()int get() in MyIntSupplier).
      • Re-ordered annotations/fields for clarity.
      • Replaced assertTrue(!condition) with assertFalse(condition) and == with assertSame where identity is being asserted.
      • Switched to more explicit StandardCharsets.UTF_8 usage when constructing PrintStreams and decoding streams in tests, improving cross-platform determinism.
    • Updated various tests to use more precise messages and to avoid anonymous Callable imports via fully-qualified types, aligning with Checkstyle/SpotBugs expectations.

Overall, this PR strengthens the cached compiler’s threading and I/O story, aligns requirements and decisions with a traceable Nine-Box taxonomy, and gives AI agents and human contributors clearer, security-aware guidance on how to work within the Java Runtime Compiler module.

@sonarqubecloud
Copy link

@peter-lawrey peter-lawrey changed the title Adv/develop Harden Java runtime compiler caching, I/O safety, and documentation/agent guidance Nov 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants