nim r --d:release --mm:atomicArc thisexample
And we should make it the default.
And we should make it the default.
ugh no thanks - making the entire codebase thread-synchronized because (in a well-designed application) you have a few transfer points between threads is not gonna end well.
Great news! Why does your getandpost.nim example only use epolldispatcher for the POST requests/server and not the GET req/server?
BTW, thanks for all the helpful info in the release notes for v8.
No reason. Example 2 merely tries to demonstrate the central tenet of GuildenStern, namely that you can run, switch, and customize easily between different kinds of servers in different ports so that there is optimized handling available for any kind of client workflow pattern your application might encounter.
If there is no need for multiple dispatcher logic in same application, you can definitely stick to one. If your platform supports it, I recommend trying out the new epolldispatcher, because it delegates scheduling to kernel and should therefore be a robust choice.
When I run stress tests with atomicArc, I get rock-solid performance without a noticeable degradation in speed. So, yes, I will use atomicArc as my default everywhere, at least until I learn to prevent gc from crashing by using those advanced tools @arnetheduck is referring to.
When I run nim examples/wsmulticasttest with --mm:arc, the program sooner later hits something like this:
/home/allin/.choosenim/toolchains/nim-2.2.0/lib/system.nim(180) handleRead
(sometimes, only with useMalloc:) free(): double free detected in tcache 2
...
/home/allin/.choosenim/toolchains/nim-2.2.0/lib/system/arc.nim(196) nimDestroyAndDispose
(sometimes, without useMalloc:) /home/allin/.choosenim/toolchains/nim-2.2.0/lib/system/arc.nim(229) nimDecRefIsLastIf someone knows a fix for that offending proc, help would be appreciated. In the meantime, atomicArc is my friend.
I recently just dove into threadsanitizer, and it really helped me find some places where ARC + temporary "hidden" copies were crashing my threaded code. Also use debugger:native and it'll use the itanium name mangling and give you a better stacktrace.
I got curious. Looks like you're passing GuildenStern ref object to threads. That'd cause ARC thread issues even if the object isn't freed as there can still be increment and decrements. You could possibly turn "server" into a plain object and put it into a SharedPtr frim threading/smartptrs. If you own the object that's easiest thing to do. Lots of other ways too.
That's if you wanna use regular ARC. Atomic Arc mostly gets expensive only if there's lots of contention. Doesn't look like you'll have that.
Thanks for taking a look. The root cause is simple: when --threads:on became the default, I deduced that Nim runtime would be threadsafe, and started using refs more, instead of pointers.
So basically the atomicArc is what I supposed the "plain" arc to already offer, and that is what I would like to see as the default (well, "atomicOrc" really). A language run-time should be safe and trustable. The developer may explicitly choose to add "footguns" by using compiler switches, experimental features, pragmas and whatnot, not the other way around. But discussing programming language ergonomy trade-offs belongs to some other thread (sic)...
Concerning GuildenStern, the offending procedure does indeed assign a ref, so that is the problem when gc ref counting is not thread-safe. I decided to fix this by using ptr instead of ref: it makes sense, because servers live to the end of the program and therefore all reference counting is futile anyway. So far so good, everything seems to work correctly now also with the default memory managers (just few ugly allocs and casts here and there, but fortunately not visible to end user). The fixed code is at devel branch, and if all looks good, will be published soon.
Again, thanks for all support.
import "streams"
var f = newStringStream()
proc a() =
{.gcsafe.}:
echo f != nil
var t: Thread[void]
createThread(t, a)
echo isUniqueRef(f)
f = nilThe last line echoes true.
unless you are creating the ref at the time of the transfer
this is the key point - a reference has a single owner at creation - most references, if you move things around, continue to have a single owner - the language however offers no natural way to express this constraint to its detriment. See C++ for example where unique_ptr is the default memory manager for references and you can decay it into a shared_ptr if you're in one of those magic exceptional moments that merit an escape hatch.
Something is wrong with the current ref.
indeed - it lacks in expressivity, ie it can't capture the concept of single ownership and/or transfer. It's as if the language didn't allow you to declare an int, but only seq[int] - sure, you can write code in such a language but oh boy are you in for a ride.
The same thing with references and owners - single-owner references are fundamentally more simple to reason about (also in non-threaded code! but threading exacerbates the issues) and this reflects back on the complexity of the code you have to write (if you want to stay "correct") and/or the amount of bugs you're willing to accept. Even with lipstick, this will remain a pig ;)
I'm not going to spend a bunch of time creating a proof, but it is pretty clear to see how a recursive, not-locked non-atomic read visiting many objects is clearly not actually safe. All that needs to happen is the ref count of any object in the tree change while the recursion is happening.
Of course it's not safe for a reference already shared across threads. You've already violated the constraint of ARC at that point and will have potential race conditions in your reference counts.
To clarify, the purpose of isUniqueRef can only be for ensuring that you can safely move or send a ref to another thread using a channel or other safe mechanism where you give up ownership of the ref.
There's no way to share non-atomic ref between threads safely, read-only or not. runtimeIsolate can only tell you that a given ref only has a single (local) owner which you can then give to another thread.
I'm not sure what @arenetheduck single-owner ref implementation would look like exactly, but it would help ensure that both isUniqueRef is always true without a runtime check and that the ownership is properly moved. It would still be invalid in your given scenario and presumably could be similarly overridden.
Both echoes are true. f is accessed from multiple threads so isUniqueRef is dangerous to trust as meaning "other threads cannot access this".
The mechanism that does protect "from other threads cannot access this" is gcsafe. IMHO, it's working as intended as you had to explicitly do a cast to get it to compile.
proc a() =
{.gcsafe.}: # You've explicitly broken thread safety here for non-atomic refs
echo f != nilDoing that is sort of analogous to saying you can't trust a GC when you're manually doing casts and manual pointer arithmetic. ;) In that scenario it's on the programmer to verify what they're doing.
You can also accomplish this in Rust using unsafe to violate it's single-owner enforcement as well.
I said: "with an unsafe ref it is never possible to safely transfer ownership without the programmer asserting it is"
You said: "Side note that's not quite true; it's possible but you do have to pay a runtime cost to do a isUniqueRef check."
I demonstrated this false.
Now you say "Of course it's not safe for a reference already shared across threads."
Ok, well we're back to what I said. It's not actually safe unless the programmer asserts it is.
Ok, well we're back to what I said. It's not actually safe unless the programmer asserts it is.
Well that's true, but it's a tautology. If it's an unsafe ref, then by definition the programmer needs to assert it's safe.
In my mind goes to what's the point in saying "Also regarding 'the tool you need for that is single-ownership (with transfer / isolation)'" if you immediately switch to a talking about an unsafe ref which violates that safety.
I reckon I'm still confused as to the point in your original comment then. :)
The point of the whole chain of comments is that ref is not usable across threads except in extremely limited and carefully orchestrated ways. This is not true with atomicArc. It is also true that atomicArc is what @Allin expected, and is what I think is reasonable to expect.
There is no need of "isolated" if there is atomicArc, there is just the programmer knowing what is and is not safe, which is always true.
isUniqueRef seems to be of no value given my example above. Not sure what it proves.
I think you’re missing what isUniqueRef actually provides and it’s purpose.
A non atomic ref isn’t automatically “unsafe” after using it and not isolating it immediately. That’s unsafe in the sense that isUniqueRef becomes invalid. It’s only that the compiler can’t prove you didn’t take local refs and there’s not a single (local) owner. Arnetheduck’s single ownership types would enable that but restrict the ref in other ways.
Unique refs only encounter the problem you describe above after you use cast-gcsafe to circumvent the safety and share it unsafely with another thread. Sharing an Isolated[Asdf] would also break.
Avoid cast-gcsafe and then using isUniqueRef remains sound. Then runtimeIsolate works and tells you that there’s only one (local) owner of a ref or not and therefore if it’s safe to give to another thread.
Here uniqueness refers only to local handles of a ref, not whether it’s already shared with other threads.
First class single ownership types would have the same problem as well.
Now I do agree that isolated and single ownership are limiting in a lot of cases. Also atomic refs are easier to use regardless of mutability and locking. But isolated and isUniqueRef let you safely do threading. It’s prevented real concurrency bugs for me.
It’s probably time to move this discussion to another thread. :) Maybe I’ll try write up a blog post and try to describe the proper usage of isUniqueRef and isolated though and folks can dissect that.
It’s probably time to move this discussion to another thread. :) Maybe I’ll try write up a blog post and try to describe the proper usage of isUniqueRef and isolated though and folks can dissect that.
Yes, please. This is all a very interesting discussion, but - apart from msgs #6 and #7 has strayed quite far from the original topic.
@mods Is it possible to move the existing discussion to a separate topic?