vy commented on code in PR #2517:
URL: https://github.com/apache/logging-log4j2/pull/2517#discussion_r1580860904


##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/Provider.java:
##########


Review Comment:
   > 
https://github.com/apache/logging-log4j2/blob/cc876ae7c629b698b92414b7f3c94213cdd0a427/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AppenderControl.java#L39
   > 
   > I came to the conclusion that our `log4j2.enableThreadlocals` contract can 
be: "if the value is `false` we **do** use `ThreadLocal`s, but we clear them up 
before returning to user code".
   
   You are saying _"if `log4j2.enableThreadlocals=false` we **do** use 
`ThreadLocal`s but..."_ sorry but this makes no sense. No matter how much you 
stretch the meaning of the words, the intent is crystal clear.
   
   > The `ThreadContext` class is not different,
   
   It is completely different. You are comparing apples and oranges. If a user 
calls `ThreadContext`, by design, they expect the context propagation 
behaviour, and _the one and only way_ to achieve this is using `ThreadLocal`s. 
(Even the newly introduced `ScopedValue` uses `TL`s!) You cannot say _"I want 
to use `ThreadContext` but please don't store anything in the `Thread`"_.



-- 
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