Skip to content

Make accesses to __iNtErNaL_running_procs thread-safe#104

Merged
pankgeorg merged 1 commit intoJuliaPluto:mainfrom
giordano:mg/thread-safety
Mar 27, 2026
Merged

Make accesses to __iNtErNaL_running_procs thread-safe#104
pankgeorg merged 1 commit intoJuliaPluto:mainfrom
giordano:mg/thread-safety

Conversation

@giordano
Copy link
Contributor

__iNtErNaL_running_procs is a global state, and in a multi-threaded scenario it's unsafe to directly push! to it. A simple lock around it solves the issue. Without this, I got occasional

  Got exception outside of a @test
  TaskFailedException
      nested task error: AssertionError: Multiple concurrent writes to Dict detected!
      Stacktrace:
        [1] rehash!(h::Dict{Base.Process, Nothing}, newsz::Int64)
          @ Base ./dict.jl:183
        [2] ht_keyindex2_shorthash!(h::Dict{Base.Process, Nothing}, key::Base.Process)
          @ Base ./dict.jl:270
        [3] setindex!(h::Dict{Base.Process, Nothing}, v0::Nothing, key::Base.Process)
          @ Base ./dict.jl:358
        [4] push!
          @ ./set.jl:137 [inlined]
        [5] Malt.Worker(; env::Vector{Pair{String, String}}, exename::String, exeflags::Vector{String}, monitor_stdout::Bool, monitor_stderr::Bool)
          @ Malt ~/.julia/packages/Malt/yA40d/src/Malt.jl:137
        [6] Worker
          @ ~/.julia/packages/Malt/yA40d/src/Malt.jl:114 [inlined]

@pankgeorg
Copy link
Member

I'm a bit concerned that all our tests are red. The ones on pre and nightly are red -I think- mostly because there have been significant changes to cancelling tasks (we kind of use undocumented functions, oops). Could you help us look into the others? Beyond this PR of course!

@giordano
Copy link
Contributor Author

Yes, but they're failing already in main (and already before my previous PR 😅)

Could you help us look into the others?

To be helpful, I'd need to understand the motivation behind the tests. For example I don't know what

@test tend - tstart < 15.0
really wants to test.

In meantime a new version including this PR would be very welcome downstream 😃

@giordano
Copy link
Contributor Author

To be helpful, I'd need to understand the motivation behind the tests. For example I don't know what

@test tend - tstart < 15.0
really wants to test.

I decided to give it a go because of lack of sleep, so I went down a rabbit hole trying to decipher what the test was really trying to assert, and the result of my investigation is #105. This doesn't solve the problems with Julia nightly, but at least there should be fewer red marks on CI.

@pankgeorg pankgeorg merged commit 97b5e08 into JuliaPluto:main Mar 27, 2026
1 of 19 checks passed
@pankgeorg
Copy link
Member

I decided to give it a go because of lack of sleep,

I really appreciate and hope the decision did not cause more lack of sleep!

@giordano giordano deleted the mg/thread-safety branch March 27, 2026 08:17
@giordano
Copy link
Contributor Author

@pankgeorg could you please register v1.4.1? Thanks!

@pankgeorg
Copy link
Member

pankgeorg commented Mar 27, 2026

@pankgeorg could you please register v1.4.1? Thanks!

JuliaRegistries/General#151450 !!!

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 this pull request may close these issues.

3 participants