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

Vendor ReadWriteLock from ConcurrentUtils #85

Merged
merged 1 commit into from
Apr 4, 2024
Merged

Conversation

jpsamaroo
Copy link
Collaborator

Vendors the ConcurrentUtils.ReadWriteLock to work around Windows incompatibility of Try.jl, until that can be forked to a new organization and issues resolved. In this case, ReadWriteLock only needs UnsafeAtomics.jl (which has active maintainers), so we depend on that instead.

Should fix #84. @DrChainsaw can you validate this?

@jpsamaroo jpsamaroo added the bug label Apr 4, 2024
@DrChainsaw
Copy link
Contributor

Thanks alot for the quick turnaround!

Can confirm that I can install this branch on my machine.

However, I also ran the Dagger test suite locally with the branch and got two ConcurrencyViolationError errors which may or may not be related. Here is the first one:

per-call: Error During Test at .julia\dev\Dagger\test\thunk.jl:65
  Test threw exception
  Expression: fetch(c) == 20
  ThunkFailedException:
    Root Exception Type: CapturedException
    Root Exception:
  ConcurrencyViolationError("lock must be held")
  Stacktrace:
    [1] concurrency_violation
      @ .\condition.jl:8
    [2] assert_havelock
      @ .\condition.jl:25 [inlined]
    [3] assert_havelock
      @ .\condition.jl:48 [inlined]
    [4] assert_havelock
      @ .\condition.jl:72 [inlined]
    [5] _wait2
      @ .\condition.jl:83
    [6] #wait#645
      @ .\condition.jl:127
    [7] wait
      @ .\condition.jl:125 [inlined]
    [8] wait_for_conn
      @ .julia\juliaup\julia-1.10.2+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Distributed\src\cluster.jl:195
    [9] check_worker_state
      @ .julia\juliaup\julia-1.10.2+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Distributed\src\cluster.jl:170
   [10] send_msg_
      @ .julia\juliaup\julia-1.10.2+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Distributed\src\messages.jl:172
   [11] send_msg
      @ .julia\juliaup\julia-1.10.2+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Distributed\src\messages.jl:122 [inlined]
   [12] #remotecall_fetch#159
      @ .julia\juliaup\julia-1.10.2+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Distributed\src\remotecall.jl:460
   [13] remotecall_fetch
      @ .julia\juliaup\julia-1.10.2+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Distributed\src\remotecall.jl:454
   [14] remotecall_fetch
      @ .julia\juliaup\julia-1.10.2+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Distributed\src\remotecall.jl:492
   [15] #192
      @ .julia\packages\MemPool\EYA1u\src\datastore.jl:527
   [16] forwardkeyerror
      @ .julia\packages\MemPool\EYA1u\src\datastore.jl:503
   [17] poolget
      @ .julia\packages\MemPool\EYA1u\src\datastore.jl:526
   [18] move
      @ .julia\dev\Dagger\src\chunks.jl:98
   [19] move
      @ .julia\dev\Dagger\src\chunks.jl:96 [inlined]
   [20] move
      @ .julia\dev\Dagger\src\chunks.jl:102
   [21] #invokelatest#2
      @ .\essentials.jl:892 [inlined]
   [22] invokelatest
      @ .\essentials.jl:889 [inlined]
   [23] #166
      @ .julia\dev\Dagger\src\sch\Sch.jl:1554
  Stacktrace:
   [1] wait
     @ .\task.jl:352 [inlined]
   [2] fetch
     @ .\task.jl:372 [inlined]
   [3] fetch_report
     @ .julia\dev\Dagger\src\sch\util.jl:263
   [4] do_task
     @ .julia\dev\Dagger\src\sch\Sch.jl:1563
   [5] #143
     @ .julia\dev\Dagger\src\sch\Sch.jl:1303
    This Thunk:  Thunk(id=13, *(Thunk[11](+, ...), Thunk[12](sum, ...)))
  Stacktrace:
    [1] fetch(t::Dagger.ThunkFuture; proc::OSProc, raw::Bool)
      @ Dagger .julia\dev\Dagger\src\eager_thunk.jl:16
    [2] fetch
      @ .julia\dev\Dagger\src\eager_thunk.jl:11 [inlined]
    [3] #fetch#73
      @ .julia\dev\Dagger\src\eager_thunk.jl:65 [inlined]
    [4] fetch(t::Dagger.EagerThunk)
      @ Dagger .julia\dev\Dagger\src\eager_thunk.jl:61
    [5] macro expansion
      @ .julia\juliaup\julia-1.10.2+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Test\src\Test.jl:669 [inlined]
    [6] macro expansion
      @ .julia\dev\Dagger\test\thunk.jl:65 [inlined]
    [7] macro expansion
      @ .julia\juliaup\julia-1.10.2+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Test\src\Test.jl:1577 [inlined]
    [8] macro expansion
      @ .julia\dev\Dagger\test\thunk.jl:57 [inlined]
    [9] macro expansion
      @ .julia\juliaup\julia-1.10.2+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Test\src\Test.jl:1577 [inlined]
   [10] top-level scope
      @ .julia\dev\Dagger\test\thunk.jl:52

@jpsamaroo
Copy link
Collaborator Author

Great, thanks for testing! Those concurrency violation errors are from Distributed - you need JuliaLang/Distributed.jl#4 and Julia 1.11+ if you want to use Distributed with multithreading.

@jpsamaroo jpsamaroo merged commit 9b2504b into master Apr 4, 2024
3 of 6 checks passed
@jpsamaroo jpsamaroo deleted the jps/rwlock-vendor branch April 4, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.4.7 can't be installed on windows
2 participants