Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Julia 1.11 "Multiple concurrent writes to Dict detected!" middleware_cache #226

Closed
arlowhite opened this issue Oct 9, 2024 · 2 comments · Fixed by #227
Closed

Julia 1.11 "Multiple concurrent writes to Dict detected!" middleware_cache #226

arlowhite opened this issue Oct 9, 2024 · 2 comments · Fixed by #227
Assignees

Comments

@arlowhite
Copy link

arlowhite commented Oct 9, 2024

After updating to Julia 1.11.0 I've started to see this error. I never saw it with Julia 1.10.5

Oxygen v1.5.13

┌ Error: ERROR: 
│   exception =
│    AssertionError: Multiple concurrent writes to Dict detected!
│    Stacktrace:
│      [1] rehash!(h::Dict{String, Function}, newsz::Int64)
│        @ Base .\dict.jl:182
│      [2] ht_keyindex2_shorthash!(h::Dict{String, Function}, key::String)
│        @ Base .\dict.jl:268
│      [3] setindex!(h::Dict{String, Function}, v0::Function, key::String)
│        @ Base .\dict.jl:356
│      [4] (::Oxygen.Core.Middleware.var"#2#4"{Oxygen.Core.var"#31#34"{HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Bool, Bool}, HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Vector{typeof(ReefGuideAPI.CorsMiddleware)}, Dict{String, Tuple}, Dict{String, Function}})(req::HTTP.Messages.Request)
│        @ Oxygen.Core.Middleware C:\Users\awhite\.julia\packages\Oxygen\RkoDD\src\middleware.jl:61
│      [5] (::Oxygen.Core.var"#38#42"{HTTP.Messages.Request, Oxygen.Core.Middleware.var"#2#4"{Oxygen.Core.var"#31#34"{HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Bool, Bool}, HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Vector{typeof(ReefGuideAPI.CorsMiddleware)}, Dict{String, Tuple}, Dict{String, Function}}, Oxygen.Core.AppContext.Service})()
│        @ Oxygen.Core C:\Users\awhite\.julia\packages\Oxygen\RkoDD\src\core.jl:355
│      [6] handlerequest(getresponse::Oxygen.Core.var"#38#42"{HTTP.Messages.Request, Oxygen.Core.Middleware.var"#2#4"{Oxygen.Core.var"#31#34"{HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Bool, Bool}, HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Vector{typeof(ReefGuideAPI.CorsMiddleware)}, Dict{String, Tuple}, Dict{String, Function}}, Oxygen.Core.AppContext.Service}, catch_errors::Bool; show_errors::Bool)
│        @ Oxygen.Core.Util C:\Users\awhite\.julia\packages\Oxygen\RkoDD\src\utilities\misc.jl:45
│      [7] handlerequest
│        @ C:\Users\awhite\.julia\packages\Oxygen\RkoDD\src\utilities\misc.jl:40 [inlined]
│      [8] #37
│        @ C:\Users\awhite\.julia\packages\Oxygen\RkoDD\src\core.jl:352 [inlined]
│      [9] (::Oxygen.Core.var"#26#28"{Oxygen.Core.var"#37#41"{Oxygen.Core.Middleware.var"#2#4"{Oxygen.Core.var"#31#34"{HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Bool, Bool}, HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Vector{typeof(ReefGuideAPI.CorsMiddleware)}, Dict{String, Tuple}, Dict{String, Function}}, Oxygen.Core.AppContext.Service, Bool}, HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, String})(req::HTTP.Messages.Request)
│        @ Oxygen.Core C:\Users\awhite\.julia\packages\Oxygen\RkoDD\src\core.jl:325
│     [10] #9
│        @ C:\Users\awhite\.julia\packages\Oxygen\RkoDD\src\core.jl:185 [inlined]
│     [11] (::HTTP.Handlers.var"#1#2"{Oxygen.Core.var"#9#11"{Oxygen.Core.var"#26#28"{Oxygen.Core.var"#37#41"{Oxygen.Core.Middleware.var"#2#4"{Oxygen.Core.var"#31#34"{HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Bool, Bool}, HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Vector{typeof(ReefGuideAPI.CorsMiddleware)}, Dict{String, Tuple}, Dict{String, Function}}, Oxygen.Core.AppContext.Service, Bool}, HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, String}, Sockets.IPv4, HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.Connections.Connection{Sockets.TCPSocket}}}})(stream::HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.Connections.Connection{Sockets.TCPSocket}})
│        @ HTTP.Handlers C:\Users\awhite\.julia\packages\HTTP\sJD5V\src\Handlers.jl:58
│     [12] #12
│        @ C:\Users\awhite\.julia\packages\Oxygen\RkoDD\src\core.jl:202 [inlined]
│     [13] (::Oxygen.Core.var"#16#19"{HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.Connections.Connection{Sockets.TCPSocket}}, Oxygen.Core.var"#12#13"{Oxygen.Core.var"#26#28"{Oxygen.Core.var"#37#41"{Oxygen.Core.Middleware.var"#2#4"{Oxygen.Core.var"#31#34"{HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Bool, Bool}, HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, Vector{typeof(ReefGuideAPI.CorsMiddleware)}, Dict{String, Tuple}, Dict{String, Function}}, Oxygen.Core.AppContext.Service, Bool}, HTTP.Handlers.Router{typeof(HTTP.Handlers.default404), typeof(HTTP.Handlers.default405), Nothing}, String}}})()
│        @ Oxygen.Core C:\Users\awhite\.julia\packages\Oxygen\RkoDD\src\core.jl:216
└ @ Oxygen.Core.Util C:\Users\awhite\.julia\packages\Oxygen\RkoDD\src\utilities\misc.jl:48

.julia\packages\Oxygen\RkoDD\src\middleware.jl:61
middleware_cache[key] = strategy

I don't see it once the Julia session is warmed-up (if I restart server in same REPL session). But it happens every time during the first server start in a fresh Julia session. I'm hitting the server with multiple requests simultaneously. Running in parallel mode, -t auto,1

Let me know if you need more information.

Impact

The request encountering this error will get a HTTP 500 response

@frankier
Copy link
Contributor

It's probably fine to have a cache per task, so https://docs.julialang.org/en/v1/base/parallel/#Base.task_local_storage-Tuple{Any} could help here.

The cache is probably read heavy, so it may be slightly more efficient to add a multiple reader/single writer lock and have the cache be centralized. However, it's only better if acquiring a read lock is really cheap. This implementation https://juliaconcurrent.github.io/ConcurrentUtils.jl/dev/#ConcurrentUtils.ReadWriteLock appears correct. Probably it's better to stick with the 1st option to avoid complexity.

@ndortega
Copy link
Member

Hi @arlowhite,

Thanks for the error logs and description. In your code you're hitting the same route multiple times before the cache is populated. In this scenario multiple threads are trying to write to the same location in cache at the same time. This "middleware_cache" stores the fully composed middleware chain for each route - so we don't have to recompute it for each request.

Normally I wouldn't argue for using locks - which can introduce a decent amount of overhead, but in this case I think it would be fine to use a reentrant lock to protect the writes to this dictionary since the writes only happens once for each route.

@frankier ,
thanks for the suggestions, I didn't know tasks had a local storage operation. Since oxygen handles each request within it's own task, I'm not sure if caching at this level would have any meaningful effect. I also don't think we need to utilize that read-write lock in this scenario since the writes only happens once and then the same data can be read concurrently without locking. Granted, this kind of lock could be helpful in the metrics middleware which I believe does lock on reads and writes.

@ndortega ndortega self-assigned this Oct 15, 2024
@ndortega ndortega linked a pull request Oct 15, 2024 that will close this issue
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 a pull request may close this issue.

3 participants