borinquenkid commented on PR #15993:
URL: https://github.com/apache/grails-core/pull/15993#issuecomment-5095579590

   @jdaugherty — some findings that might help with your investigation into 
whether this can safely work.
   
   I searched git history broadly for the historical comment/reason you're 
remembering (grepped commit messages and blamed the touched files - 
`GrailsConsole.java`, `SpringApplicationRunner.java`, `GrailsCli.groovy`) and 
couldn't turn up a specific citation. If you find it, I'd like to see it too.
   
   What I did instead: identified the most plausible place the "edge case" 
could bite, and tested it directly. `SpringApplicationRunner.RunThread` 
deliberately runs with the *compiled application's own classloader* as its 
context classloader (its own Javadoc: "Thread used to launch the Spring 
Application with the correct context classloader"). SLF4J's provider discovery 
is classloader-sensitive, so a logger call whose *first-ever* SLF4J touch in 
the JVM happens from inside that thread is exactly the kind of scenario that 
could pick an ambiguous/wrong provider.
   
   That doesn't happen here, structurally: `LOG` on `SpringApplicationRunner` 
is a `static final` field, so SLF4J's one-time provider binding is triggered 
when the class is first initialized - which happens when `new 
SpringApplicationRunner(...)` is constructed on the main CLI thread, before any 
`RunThread` with a foreign classloader is ever spawned. By the time 
`RunThread.run()` calls `logOrPrintStackTrace`, the logger is already resolved.
   
   I verified this empirically rather than just reasoning about it: ran 
`SpringApplicationRunnerSpec` (6/6 pass), which forks a real JVM and exercises 
exactly this path - launch/reload/shutdown failures thrown from inside 
`RunThread` and logged via SLF4J. Also ran the full stated verification suite 
(`grails-bootstrap`, `grails-core`, `grails-shell-cli`, 
`grails-data-hibernate5`, `grails-data-hibernate7`, `grails-forge-core`) clean 
on a fresh `--rerun-tasks` run.
   
   Separately, I confirmed and fixed the multi-SLF4J-provider classpath issue 
Copilot flagged (`grails-shell-cli`'s test classpath had both `slf4j-simple` 
and `logback-classic` on it simultaneously with no exclusivity) and rebased 
onto current `8.0.x`, which also surfaced a real semantic merge conflict: 
`ExecutionContext`/`SchemaExportCommand` moved packages upstream as part of 
unrelated work, and the two new `SchemaExportCommandSpec` files (added by this 
PR, so git had no history to rename-track through) needed their imports updated 
- now fixed.
   
   This is evidence for your review, not a claim that the question is settled - 
happy to dig further if you want a different angle tested.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to