This compiles and runs fine with --newruntime:
# nim c --newruntime -r t.nim
import OS
type
Widget = ref object
i: int
a: array[1024, int]
var g1: seq[Widget]
proc newWidget(): owned Widget =
new result
proc addWidget(w: owned Widget) =
g1.add(w)
proc main =
var w: owned Widget
w = newWidget()
var x: Widget = w
addWidget(w)
w = nil
echo x.i
main()
So x is not a dangling ref. So addWidget() takes ownership -- but for that I had expected that g1 must be a seq[owned Widget]. So g1 can not really own the Widget, and addWidget() can not really own it also, as it is only a proc, no heap storage.
Here is what happens: After addWidget(w) the w has been moved so its destructor does not run. It's the destructor that detects dangling unowned refs. However, seq[Widget] does not own the memory either. You produced a memory leak.
The problem is that in addWidget you don't take over the ownership of w but to the compiler it looks like you do as seq.add takes a sink parameter. The implicit conversion from owned ref T to ref T is dangerous for sink parameters. Tough nut. Thanks for this example!
@Araq:
The problem is that in addWidget you don't take over the ownership of w...
I don't quite get this, as why doesn't the owned Widget by "addWidget" (which has had ownership moved) not get destroyed at the end of "addWidget"'s scope when it hasn't transferred ownership anywhere else such as to the seq, upon which destruction there then should be the dangling error for two reasons: the x reference in main and the added element to the seq, both of which are dangling?
Or is this just a bug?
@Araq: I think it is a bug, on the implicit conversion from owned to "unowned", the compiler is not destroying the owned as there is confusion over whether it's owned or not. For the OP's example, it wouldn't get destroyed anyway even if the seq contents were declared to be owned as g1 is a global and thus never destroyed (other than at the end of the program).
Consider the following example code not using globals, which use of globals might confuse things:
# testing ref/owned ref...
type
TestObj = object
val: int
Test = ref TestObj
proc `=destroy`(x: var TestObj) = echo "so we can see when it's destroyed"
proc main() =
let tst = Test(val: 42); echo "first type and pointer: ", tst.typeof, " ", cast[int](tst)
let tst1 = tst; echo "second type and pointer: ", tst1.typeof, " ", cast[int](tst1)
echo "second ref object ", tst[]
echo "end main"
main()
echo "end program"
first type and pointer: Test 139933770555464
second type and pointer: Test 139933770555464
second ref object (val: 42)
end main
end program
It works the same (incorrect) way if Test is declared as Test = owned (ref TestObj), which is really wrong as we are declaring that Test must be owned, should have been moved to the first value, not copied to the second without a unown, and everything destroyed properly at the end of main.
As I said, the compiler is getting confused over what is owned and not when it comes to ref's. I think this is related to the similar issue I reported to do with owned closures as of issue #11933 where there is the same confusion over these same automatic conversions as here over owned ref.
@Araq:
The owned ref's destructor then still would be run at the end of addWidget...
Shouldn't that be " The owned ref's destructor then still would be run at the end of seq.add...", as the owned ref's ownership would have been transferred to seq.add (g1.add(w)) due to the sink parameter so that it would be its responsibility to destroy it (as creating the temporary location, if that's how it's implemented, shouldn't transfer ownership although that seems to be what it is doing currently)?
@Sixte, @Araq; confirming the problem is with the sink parameter of g1.add:
I've modified my example code showing the problem in a post above by eliminating addWidget entirely to show the problem still exists with seq.add/g1.add, which is the real problem that sink parameters are not destroyed when owned and only a dangling is passed on by an auto assignment to an unowned.
@Sixte: You seem to be a proponent of the Pony memory model, and in being so wrapped up in it are missing the simplicity of what is being implemented here in the Nim newruntime. As @Araq once did for me, I highly recommend you read the Bacon/Dingle document thoroughly to see what was originally proposed and then go through the Nim implementation of that to see how even that model has been improved upon.
What you are doing is something like I often see in trying to teach foreigners to speak the local language here: they keep drawing parallels to how things work in English or whatever language they may already know and in so doing limit their appreciation of the full nuances of the language being learned, which is based on an entirely different history and culture.
As to the Pony language, I see the author's point, but lament the implementation complexity both as to concepts and as to required syntax to support those concepts; the Nim newruntime model is much simpler in concept and syntax and should be much easier for people new to the language to understand.
The Actor model on which the Pony concurrency model is based does have it's merits but the method of restricting ownership that Pony uses is not the only way to implement it - for instance, Erlang/Elixir do this in what I think is a more elegant way. If we want an Actor model of concurrency for Nim, I believe that it would be possible to build one as a library, and that use of newruntime could help express the ownership in a simpler way, although there might need to be some alternate means necessary to restrict mutability for passed messages. I haven't looked into it too deeply because,, while the Actor model has its advantages, it also has its limitations. What is great about Nim is that it doesn't restrict itself to any particular model, but tries to provide the tools to build whatever model is required for a given situation. While we aren't yet there for concurrency, explorations like newruntime are getting us much closer.
For many of us, we are glad that @Araq isn't using the Pony memory management model, and if he ever reverted to that, I think that many of us would be "out of here". There must be a better way that all the complexity of Pony or Rust. Most of us would rather just have some sort of automatic reference counting something like as in Swift than the complexity of those others if the newruntime doesn't work out.
What is great about Nim is that it doesn't restrict itself to any particular model, but tries to provide the tools to build whatever model is required for a given situation.
Amen. This is a major reason to use C++ and if Nim can offer it in a more approachable form, we get the right kind of people interested.
There must be a better way that all the complexity of Pony or Rust.
Hope your'e right, but formal logic could be against us here.
@Araq: and all...
Since no one else seems to have filed an issue on this, I just did as issue#12116. I further refined the problem as it has nothing to do with having global variables or seq's, but only to do with passing an owned ref to a sink proc as a sink parameter that gets correctly moved (marked with wasmoved to be zeroed) for the caller when "last use of" which sink proc then uses it to assign it to an "unowned"/"dangling" ref, the sink proc erroneously "moves"/zeroes it rather than causing a compiler error that the assignment is invalid as the "danglng" reference lives longer than its source.
The memory leak is caused because the erroneous zeroing/"moving" makes the only owned ref source copy unable to be destroyed.
Where are the (new) B/D improvements to be found in the source BTW?
@GordonBGood
The question is: What does the compiler see ? The compiler sees owned be passed to sink.
I already stated:
Despite this, an owned ref can't be passed to a non-owned field
and the "field" here is the src: sink Widget. There is no problem with your proc sinkprocUsing. There is a problem with "hidden aliasing". The caller can't now that sinkprocUsing would aliase the owned w. Then, dw may outlive w . This must not happen. But, if I'd quote:
Despite this, an owned ref can't be passed to a sink
sink would become useless in combination with owned.
@Sixte:
Where are the (new) B/D improvements to be found in the source BTW?
The things related to the newruntime are mostly in the runtime_v2.nim file, but also there are tweaks scattered through the code; a search for nimV2 will locate most of them.
There is no problem with your proc sinkprocUsing.
True, there would be no problem with sinkprocUsing if the compiler didn't have a bug and not report an error when it is assigning the passed owned to a "dangling" and moved it as if the destination were owned.
Despite this, an owned ref can't be passed to a sink
sink would become useless in combination with owned.
Once the compiler bug is fixed, an owned ref can be passed to a sink parameter, in which case there will be a compiler error if the sink proc passes it to a destination that is not an owned ref but everything will be fine if the destination is an owned ref whose lifetime outlives any dangling ref`'s that have been created. If the destination of the owned ref is outside the scope of the dangling ref's, then there will be a "Dangling ref error" when it is destroyed if the compiler can't detect the shorter lifetime and give an error first. For instance, even now, if sinkprocUsing doesn't do anything with the owned ref passed to its sink parameter as far as passing it on, it is destroyed when sinkprocUsing returns, causing the Dangling `ref error" if there were other references to w, which is the correct behaviour.
If dw were an owned ref instead of a dangling one, the current behaviour is correct and there should be no errors. as dw lives beyond any other references that would be made in the body of `main.
If dw were an owned ref instead of a dangling one, the current behaviour is correct and there should be no errors. as dw lives beyond any other references that would be made in the body of main.
... but proc sinkprocUsing(dst: var Widget; src: sink Widget) doesn`t expect an owned Widget. With rust syntax though , an annotation: proc sinkprocUsing(dst: var 'a Widget; src: sink 'a Widget) could be given. Then, the compiler could infer at callsite (main), that an owned Widget src gets destroyed via sink, but its lifetime and therefore the referenced object itself will survive in dst. Since other (non-owned) aliases of src could exist, the lifetime transfer has to be obeyed in main (bookkeeping). But then, you will unavoidably end with rust's lifetime reasoning and lifetime inference. Araq will not appreciate this, I think.
@Sixte:
... but proc sinkprocUsing(dst: var Widget; src: sink Widget) doesn't expect an owned Widget...
It seems that it is @Araq's intention that one doesn't have to force proc parameters to be owned or not and the compiler determines which they are according to "last read of" or not; this isn't the problem.
What is the problem (as @Araq stated in his first reply to this thread) is that the logic to determine whether to "move" an owned paramater or to copy it as a "dangling" is too ambiguous when the destination doesn't specify that it is owned such that when the compiler uses the same rule as it uses for proc parameters as to "last read/use of", it then incorrectly moves an owned to a "dangling".
Since only known "owned" references are destroyed and the type of dw is not owned, it is never destroyed, which causes the memory leak.
Applying the general rule of making dw automatically "lift" to be an owned on such an assignment would seem to be overly complicated and confusing, as in the case where the assignee were an element of a seq as in the Original Post (OP) the compiler (and programmer) would have to deal with the case where some seq elements are owned and others not - not something that should have to be considered.
The simplest solution is likely to only consider the owned/"dangling" (nonowned) status as declared and cause a compiler error where possible when there is a lifetime discrepancy or at least a "Dangling reference error" at tun time when the discrepancy can not be detected.
What is the problem (as @Araq stated in his first reply to this thread) is that the logic to determine whether to "move" an owned paramater or to copy it as a "dangling" is too ambiguous when the destination doesn't specify that it is owned such that when the compiler uses the same rule as it uses for proc parameters as to "last read/use of", it then incorrectly moves an owned to a "dangling".
And this is what I already wrote. An owned gets passed to a sink. But the sink is not a sink owned. Now, suddenly:
What is the problem (as @Araq stated in his first reply to this thread) is that the logic to determine whether to "move" an owned paramater or to copy it as a "dangling" is too ambiguous when the destination doesn't specify that it is owned such that when the compiler uses the same rule as it uses for proc parameters as to "last read/use of", it then incorrectly moves an owned to a "dangling".
So , either you declare sink owned, or an owned can't be passed to a sink. This is the conclusion, because ambiguity has to be avoided. There is an alternative though. The alternative works with coercions. Let's call the coercion f_owned, then my description given above (with lifetimes, Rust-style) could be written as:
proc sinkprocUsing(dst: var f_owned Widget; src: sink f_owned Widget) . The compiler then could infer that dst gets coerced to an owned , transferring ownership from src. The callee itself can either deallocate src and assign a new Widget to dst or assign src to dst (your example).
Applying the general rule of making dw automatically "lift" to be an owned on such an assignment would seem to be overly complicated and confusing,
B/D already have the "take" operator. Coercion does the same basically. They are a bit unclear though about the lifetimes of aliases of src. Do they follow now the lifetime of src or do they follow the lifetime of dst? With a strict runtime approach (the sad way) premature deallocation of dst would lead us to a runtime error. Better: lifetime of dst gets max(src,dst). Static analysis could tell us.
as in the case where the assignee were an element of a seq as in the Original Post (OP) the compiler (and programmer) would have to deal with the case where some seq elements are owned and others not - not something that should have to be considered.
An owned requires a seq[owned] to be placed in, of course.
@Sixte; You missed my point:
It seems that Nim parameters don't have to be specified as being owned and if not specified can handle either with the compiler causing the right behavior depending on what is provided; if it were required that the "ownedness" be specified, then one would need to provide proc overloads for all the combinations, leading to a lot of extra boilerplate code - IIRC, that is what Rust does. OTOH, storage locations need to be specified as owned or not and everything including use in copying/moving proc parameters must respect the "ownedness" of the source and destination.
This last is what the bug is for this one case where the source is a proc parameter when it is owned (which the compiler already knows) and the destination is not owned but the compiler is mistakenly treating it as if it were.