The remove() method would have to be called on every thread. Setting the ThreadLocal to null will cause massive problems if Log4j is installed in the parent’s ClassLoader as you would be killing the ThreadLocal for all applications, not just the one being underplayed.
ClassLoaders are fun. Ralph > On Mar 5, 2021, at 9:22 AM, Matt Sicker <[email protected]> wrote: > > There's a remove() method on ThreadLocal. > > On Fri, 5 Mar 2021 at 10:13, Volkan Yazıcı <[email protected]> wrote: >> >> Maybe thinking naively but... Can't we just set the ThreadLocal to null at >> shutdown? >> >> On Fri, Mar 5, 2021 at 5:04 PM Ralph Goers <[email protected]> >> wrote: >> >>> I am having a difficult time locating the conversation with Ceki but it >>> happened years ago either on the SLF4J or Logback lists. The serialization >>> of objects in the MDC was just one issue. As I recall the larger issue >>> related to ClassLoaders. >>> >>> We don’t have problems putting objects into a ThreadLocal because we are >>> careful about what we put there and how long it stays. In other words, we >>> control the ThreadLocal and its contents. With the ThreadContext we control >>> the ThreadLocal but we don’t control its contents. This means that if an >>> application tries to shutdown any objects left in the ThreadLocal that are >>> owned by the ClassLoader of the application will cause the undeployment of >>> the application to fail. S I recall that is the main reason Ceki decided to >>> only support Strings in the MDC when he created SLF4J. When I created the >>> Log4j 2 API I couldn’t find a flaw in that reasoning. >>> >>> Theoretically it would be possible to support primitive objects and any >>> objects owned by a parent ClassLoader so long as they don’t reference >>> anything owned by the application ClassLoader, but validating that would be >>> a nightmare. The only way I know of to support primitive objects would be >>> to provide overloaded methods for each of the types we would want to >>> support. >>> >>> Ralph >>> >>>> On Mar 5, 2021, at 5:57 AM, Remko Popma <[email protected]> wrote: >>>> >>>> I think so yes. >>>> But a quick read doesn’t show drawbacks. >>>> Maybe I’ll remember later. >>>> >>>> >>>>> On Mar 5, 2021, at 21:54, Volkan Yazıcı <[email protected]> >>> wrote: >>>>> >>>>> Are you referring to LOG4J2-1648 >>>>> <https://issues.apache.org/jira/browse/LOG4J2-1648>? >>>>> >>>>>> On Fri, Mar 5, 2021 at 1:45 PM Remko Popma <[email protected]> >>> wrote: >>>>>> >>>>>> There should be an existing JIRA that contains fairly extensive >>> analysis >>>>>> on this topic. >>>>>> >>>>>> There are some implications/drawbacks, can’t remember off the top of my >>>>>> head. >>>>>> >>>>>> Would need to look at the ticket but no time now, maybe tomorrow. >>>>>> >>>>>>>> On Mar 5, 2021, at 19:45, Volkan Yazıcı <[email protected]> >>> wrote: >>>>>>> >>>>>>> Hello, >>>>>>> >>>>>>> In the past couple of months I have received two complaints from >>> people >>>>>> who >>>>>>> want to put non-String values into the ThreadContextMap. >>>>>>> ThreadContext.put*() methods only accept String values, whereas 2 of >>> the >>>>>> 3 >>>>>>> backend maps (GarbageFreeSortedArrayThreadContextMap, >>>>>>> CopyOnWriteSortedArrayThreadContextMap) and the exposed >>> ReadOnlyStringMap >>>>>>> interface support non-String values. (The one out of 3, >>>>>>> DefaultThreadContextMap, employed when thread locals are disabled, >>> only >>>>>>> supports String values.) I want to improve this situation by >>> supporting >>>>>>> Object values in ThreadContextMap. Is this a known issue? What would >>> be >>>>>> the >>>>>>> implications of extending ThreadContextMap? I will appreciate some >>>>>> guidance >>>>>>> on this issue. >>>>>>> >>>>>>> Kind regards. >>>>>> >>>> >>> >>> >>> >
