Skip to content

Commit e2287aa

Browse files
committed
Improve thread safety and logging metadata
* make `ComponentLogger` immutable with an atomic `min_level` cache * protect rule updates and registry access with locks * make `with_min_level` task-local to avoid affecting concurrent tasks * delegate output synchronization to the underlying sink * generate stable log record IDs using `Base.CoreLogging.log_record_id` * tighten `@clogf` argument validation * remove unused group-less helper overloads * update docstrings for the new concurrency semantics
1 parent f438959 commit e2287aa

3 files changed

Lines changed: 63 additions & 58 deletions

File tree

‎src/ComponentLogging.jl‎

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
module ComponentLogging
22
using Logging
3+
using Threads: Atomic
34

45
include("PlainLogger.jl")
56
export PlainLogger
@@ -19,14 +20,16 @@ msg_to_tuple(x::Tuple) = x
1920
msg_to_tuple(x) = (x,)
2021

2122
## ComponentLogger
22-
mutable struct ComponentLogger{L<:AbstractLogger} <: AbstractLogger
23-
const rules::Dict{RuleKey,LogLevel}
24-
const sink::L
25-
min::LogLevel
23+
struct ComponentLogger{L<:AbstractLogger} <: AbstractLogger
24+
rules::Dict{RuleKey,LogLevel}
25+
sink::L
26+
lock::ReentrantLock
27+
min_level::Atomic{Int32}
2628
end
2729

2830
function ComponentLogger(rules::Dict{RuleKey,LogLevel}=Dict{RuleKey,LogLevel}((DEFAULT_SYM,) => Info); sink=ConsoleLogger(Debug))
29-
return ComponentLogger(rules, sink, minimum(values(rules)))
31+
rules = copy(rules)
32+
return ComponentLogger(rules, sink, ReentrantLock(), minimum(values(rules)).level)
3033
end
3134

3235
function ComponentLogger(nonstdrules::AbstractDict; sink=ConsoleLogger(Debug))
@@ -43,29 +46,28 @@ end
4346
function set_log_level!(logger::ComponentLogger, group, lvl::Union{Integer,LogLevel})
4447
grp = _tokey(group)
4548
lv = LogLevel(lvl)
46-
old = get(logger.rules, grp, nothing)
47-
logger.rules[grp] = lv
48-
if lv < logger.min
49-
logger.min = lv
50-
elseif old !== nothing && old === logger.min && lv > logger.min
51-
logger.min = minimum(values(logger.rules))
49+
@lock logger.lock begin
50+
old = get(logger.rules, grp, nothing)
51+
minlvl = LogLevel(logger.min_level[])
52+
if lv < minlvl
53+
logger.min_level[] = lv.level
54+
logger.rules[grp] = lv
55+
else
56+
logger.rules[grp] = lv
57+
if old !== nothing && old === minlvl && lv > minlvl
58+
logger.min_level[] = minimum(values(logger.rules)).level
59+
end
60+
end
5261
end
5362
return logger
5463
end
5564

5665
set_log_level!(logger::ComponentLogger, group, on::Bool) =
5766
set_log_level!(logger, group, on ? 0 : 1)
5867

59-
"Temporarily set the minimum level within a do-block; restore afterward even if an exception is thrown; no lock"
60-
function with_min_level(f::Function, logger::ComponentLogger, lvl::Union{Integer,LogLevel})
61-
old_lvl = logger.min
62-
logger.min = LogLevel(lvl)
63-
try
64-
f()
65-
finally
66-
logger.min = old_lvl
67-
end
68-
end
68+
"Temporarily set the minimum level for the current task within a do-block; restore afterward even if an exception is thrown"
69+
with_min_level(f::Function, logger::ComponentLogger, lvl::Union{Integer,LogLevel}) =
70+
task_local_storage(f, logger, LogLevel(lvl))
6971

7072
@inline function _effective_level(rules::Dict{RuleKey,LogLevel}, group::Union{Symbol,RuleKey})::LogLevel
7173
path = _tokey(group) #::NTuple{N,Symbol}
@@ -92,32 +94,33 @@ end
9294
end
9395
end
9496

95-
Logging.min_enabled_level(g::ComponentLogger) = g.min
97+
function Logging.min_enabled_level(g::ComponentLogger)::LogLevel
98+
lvl = get(task_local_storage(), g, nothing)::Union{Nothing,LogLevel}
99+
return lvl === nothing ? LogLevel(g.min_level[]) : lvl
100+
end
96101

97102
Logging.shouldlog(g::ComponentLogger, level, _module, group, id) =
98-
level >= _effective_level(g.rules, group)
103+
@lock g.lock level >= _effective_level(g.rules, group)
99104

100105
Logging.handle_message(logger::ComponentLogger, level::LogLevel, message, _module, group, id, file, line; kwargs...) =
101106
Logging.handle_message(logger.sink, level, message, _module, group, id, file, line; kwargs...)
102107

103108
## Module registry
104-
const _REGISTRY_LOCK = ReentrantLock()
105-
const _REGISTRY = IdDict{Module,AbstractLogger}()
109+
const _RegistryLock = ReentrantLock()
110+
const _Registry = IdDict{Module,AbstractLogger}()
106111

107112
function set_module_logger(mod::Module, logger::AbstractLogger)::String
108-
lock(_REGISTRY_LOCK) do
109-
_REGISTRY[mod] = logger
110-
end
113+
@lock _RegistryLock _Registry[mod] = logger
111114
string(mod) * " <- " * string(typeof(logger))
112115
end
113116

114117
"Get the logger for the calling module; if unbound, fallback through parent modules; error at the top"
115118
function get_logger(mod::Module)
116-
lock(_REGISTRY_LOCK) do
119+
@lock _RegistryLock begin
117120
m = mod
118121
while true
119-
if haskey(_REGISTRY, m)
120-
return _REGISTRY[m]
122+
if haskey(_Registry, m)
123+
return _Registry[m]
121124
end
122125
pm = parentmodule(m)
123126
if pm === m # reached the top (e.g. Base/Core/Main)
@@ -268,14 +271,15 @@ function _print_tree(io::IO, rules::Dict{RuleKey,LogLevel};
268271
end
269272

270273
function Base.show(io::IO, ::MIME"text/plain", logger::ComponentLogger)
274+
rules, minlvl = @lock logger.lock (copy(logger.rules), LogLevel(logger.min_level[]))
271275
println(io, "ComponentLogger")
272276
print(io, " sink:\t")
273277
println(io, nameof(typeof(logger.sink)))
274278
print(io, " min:\t")
275-
_print_level(io, logger.min)
279+
_print_level(io, minlvl)
276280
println(io)
277-
println(io, " rules:\t", length(logger.rules))
278-
_print_tree(io, logger.rules)
281+
println(io, " rules:\t", length(rules))
282+
_print_tree(io, rules)
279283
end
280284

281285
include("helpers.jl")

‎src/docstrings.jl‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,11 @@ A logger that delegates to an underlying sink (`AbstractLogger`) while applying
3333
component-based minimum level rules. Rules are defined on paths of symbols
3434
(`NTuple{N,Symbol}`). A lookup walks up the path and falls back to `(:__default__,)`.
3535
36+
- `rules`: mapping from `NTuple{N,Symbol}` to `LogLevel`. If no explicit
37+
`(:__default__,)` rule exists, lookup falls back to `Info`.
3638
- `sink`: the underlying `AbstractLogger` that actually handles messages.
37-
- `rules`: mapping from `NTuple{N,Symbol}` to `LogLevel`. The default entry
38-
`((DEFAULT_SYM,), Info)` is created automatically when needed.
39-
40-
The effective minimum level is the minimum of all values in `rules`, cached in
41-
the `min` field for fast checks.
39+
- `lock`: protects concurrent access to `rules`.
40+
- `min_level`: atomic cache of the minimum value in `rules` for fast checks.
4241
"""
4342
ComponentLogger
4443

@@ -49,15 +48,17 @@ Set or update the minimum level for a specific component `group` on `logger`.
4948
`group` may be a `Symbol` or a `NTuple{N,Symbol}` tuple; `lvl` can be `LogLevel` or `Integer`.
5049
If `lvl` is a `Bool`, it is treated as a simple switch: `true` sets the rule to `Info` and
5150
`false` sets it to `LogLevel(1)` (which disables the default `clogenabled(logger, group)` check).
52-
Updates the internal `min` cache appropriately.
51+
Updates `rules` under `logger.lock` and keeps the atomic `min_level` cache consistent.
5352
"""
5453
set_log_level!
5554

5655
"""
5756
with_min_level(f, logger, lvl)
5857
59-
Temporarily set `logger.min` to `lvl` while executing `f()`, restoring the
60-
original value afterward even if an exception is thrown.
58+
Temporarily override the minimum enabled level for the current task while
59+
executing `f()`. The override is task-local, so it does not modify `logger.min_level`
60+
or affect other concurrent tasks. Nested overrides are restored correctly, including
61+
when `f()` throws an exception.
6162
"""
6263
with_min_level
6364

@@ -232,8 +233,8 @@ clogf(:core, 0) do
232233
end
233234
set_log_level(:core, 1000)
234235
with_min_level(2000) do
235-
# Temporarily raise the global minimum level (fast early rejection).
236-
clog(:core, 0, "suppressed by global min")
236+
# Temporarily raise the current task's minimum level (fast early rejection).
237+
clog(:core, 0, "suppressed by the task-local minimum")
237238
end
238239
```
239240

‎src/helpers.jl‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ clog(logger::AbstractLogger, group::Union{Symbol,RuleKey}, message...;
1717
_module=nothing, id=nothing, file=nothing, line=nothing, kwargs...) =
1818
clog(logger, group, Info, message...; _module, id, file, line, kwargs...)
1919

20-
# clog(logger::AbstractLogger, level::Union{Integer,LogLevel}, message...;
21-
# _module=nothing, id=nothing, file=nothing, line=nothing, kwargs...) =
22-
# clog(logger, (DEFAULT_SYM,), level, message...; _module, id, file, line, kwargs...)
23-
2420
## clogenabled
2521
function clogenabled(logger::AbstractLogger, group::Union{Symbol,RuleKey}, level::Union{Integer,LogLevel})::Bool
2622
grp = _tokey(group)
@@ -31,9 +27,6 @@ end
3127
clogenabled(logger::AbstractLogger, group::Union{Symbol,RuleKey}) =
3228
clogenabled(logger, group, Info)
3329

34-
# clogenabled(logger::AbstractLogger, level::Union{Integer,LogLevel}) =
35-
# clogenabled(logger, (DEFAULT_SYM,), level)
36-
3730
## clogf
3831
@inline function clogf(f::F, logger::AbstractLogger, group::Union{Symbol,RuleKey}, level::Union{Integer,LogLevel}; _module=nothing, file=nothing, line=nothing)::Nothing where {F<:Function}
3932
grp = _tokey(group)
@@ -107,14 +100,17 @@ macro clog(args...)
107100

108101
msg_tuple = Expr(:tuple, map(esc, msg_ps)...)
109102
mod, file, line = Base.CoreLogging.@_sourceinfo
103+
id = Base.CoreLogging.log_record_id(mod, lvl_ex, Expr(:tuple, msg_ps...),
104+
(Expr(:(=), :_group, grp_ast),))
110105
return :(
111106
let CL = ComponentLogging,
112107
_lg = ComponentLogging.get_logger($mod),
113108
_lvl = ComponentLogging.LogLevel($(esc(lvl_ex))),
114-
_grp = $grp_ast
109+
_grp = $grp_ast,
110+
_id = $(QuoteNode(id))
115111

116-
if CL._enabled(_lg, _lvl, _grp; _module=$mod, id=nothing)
117-
CL.Logging.handle_message(_lg, _lvl, $msg_tuple, $mod, _grp, nothing, $file, $line)
112+
if CL._enabled(_lg, _lvl, _grp; _module=$mod, id=_id)
113+
CL.Logging.handle_message(_lg, _lvl, $msg_tuple, $mod, _grp, _id, $file, $line)
118114
end
119115
nothing
120116
end
@@ -158,29 +154,33 @@ macro clogf(args...)
158154
error("@clogf: group must be a literal Symbol like :core or a tuple of literal Symbols like (:a,:b)")
159155
end
160156
grp_ast = is_sym_lit(args[1]) ? Expr(:tuple, args[1]) : args[1]
161-
n >= 3 || error("@clogf: with group, need (group, level, expr)")
157+
n == 3 || error("@clogf: with group, need exactly (group, level, expr)")
162158
lvl_ex = args[2]
163159
body_ex = args[3]
164160
else
161+
n == 2 || error("@clogf: need exactly (level, expr) or (group, level, expr)")
165162
lvl_ex = args[1]
166163
body_ex = args[2]
167164
end
168165

169166
mod, file, line = Base.CoreLogging.@_sourceinfo
167+
id = Base.CoreLogging.log_record_id(mod, lvl_ex, body_ex,
168+
(Expr(:(=), :_group, grp_ast),))
170169
return :(
171170
let CL = ComponentLogging,
172171
_lg = ComponentLogging.get_logger($mod),
173172
_lvl = ComponentLogging.LogLevel($(esc(lvl_ex))),
174-
_grp = $grp_ast
173+
_grp = $grp_ast,
174+
_id = $(QuoteNode(id))
175175

176-
if CL._enabled(_lg, _lvl, _grp; _module=$mod, id=nothing)
176+
if CL._enabled(_lg, _lvl, _grp; _module=$mod, id=_id)
177177
_msg = $(esc(body_ex))
178178
if _msg isa Function
179179
_msg = _msg()
180180
end
181181
if _msg !== nothing
182182
CL.Logging.handle_message(_lg, _lvl, CL.msg_to_tuple(_msg),
183-
$mod, _grp, nothing, $file, $line)
183+
$mod, _grp, _id, $file, $line)
184184
end
185185
end
186186
nothing

0 commit comments

Comments
 (0)