Skip to content

Channels do not deep copy with gc:orc contrary to the docs #24856

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

Open
yglukhov opened this issue Apr 9, 2025 · 3 comments
Open

Channels do not deep copy with gc:orc contrary to the docs #24856

yglukhov opened this issue Apr 9, 2025 · 3 comments
Labels
Documentation Related to documentation content (not generation).

Comments

@yglukhov
Copy link
Member

yglukhov commented Apr 9, 2025

Nim Version

Nim Compiler Version 2.3.1 [Linux: amd64]
Compiled at 2025-04-09
Copyright (c) 2006-2025 by Andreas Rumpf

git hash: 29a2e25
active boot switches: -d:release

Description

type
  Msg = object
    content: Content
  Content = ref object
    value: int

var chan: Channel[Msg]

var contentAddress: int

# This proc will be run in another thread using the threads module.
proc firstWorker() =
  let content = Content(value: 5)
  contentAddress = cast[int](content)
  echo "Sending content: ", contentAddress
  chan.send(Msg(content: content))

# Initialize the channel.
chan.open()

# Launch the worker.
var worker1: Thread[void]
createThread(worker1, firstWorker)

# Block until the message arrives, then print it out.
let c = chan.recv()
echo "Received content: ", cast[int](c.content)
doAssert(contentAddress != cast[int](c.content)) # FAILS HERE!!!

# Wait for the thread to exit
worker1.joinThread()

Current Output

Sending content: 139901182169160
Received content: 139901182169160
/home/y/proj/test/tests/test2.nim(160) test2
/home/y/proj/nim-devel/lib/std/assertions.nim(41) failedAssertImpl
/home/y/proj/nim-devel/lib/std/assertions.nim(36) raiseAssert
/home/y/proj/nim-devel/lib/system/fatal.nim(53) sysFatal
Error: unhandled exception: /home/y/proj/test/tests/test2.nim(160, 9) `contentAddress != cast[int](c.content)`  [AssertionDefect]

Expected Output

No assertsion failure

Known Workarounds

No response

Additional Information

Behaves correctly with --gc:refc

@Araq
Copy link
Member

Araq commented Apr 9, 2025

Docs need to be updated. One point of ORC and the shared MM is that we can send without deepcopies.

@Araq Araq added the Documentation Related to documentation content (not generation). label Apr 9, 2025
@yglukhov
Copy link
Member Author

yglukhov commented Apr 9, 2025

But then how the reference counting will work for ref objects shared between two threads? It is not atomic, is it?

@Araq
Copy link
Member

Araq commented Apr 9, 2025

With --mm:atomicArc it is. If it's not you move the references around and then you need to ensure uniqueness for this to work. IME it's easy to do that in practice but almost impossible to come up with good compiler support for it. Well there is isolate...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Related to documentation content (not generation).
Projects
None yet
Development

No branches or pull requests

2 participants