Modular•16mo ago

Running twice gives error

i've been trying to figure this out for a few days now, it's probably a memory leak, but i'm not sure. i'm hoping someone can give it a look over and lead me down the right track. heres the playground link: https://playground.modular.com/user-redirect/?hubshare-preview=OTA0YjMzOWMzMjI5MzllNjQwYTUxYTgyYjg3M2YwMzFhNTVkMmQ5MzU3OWUxOWFiMmFkMGU0NmJlZDA5NWFhNi9VbmZvbGRlcjJlbGVjdHJpY2Jvb2dhbG9vLmlweW5i

Modular Developer Console

55 Replies

HelehexOP•16mo ago

sometimes, when i hold my tongue at the right angle, it gives the correct output for step [1,1,1], so i know the proccess is mostly working

HelehexOP•16mo ago

message.txt

TeamPuzel•16mo ago

It seems to me like you have a copy constructor and a move constructor. I don't see you using the ^ operator so you seem to be copying these a lot You seem to be doing reference counting so I don't think that's a memory leak but you array accessors don't seem to check the reference count Are you intentionally trying to achieve shared mutable state here? Because that can lead to very unpredictable behavior as you modify the same underlying storage in multiple places

ModularBot•16mo ago

Congrats @TeamPuzel, you just advanced to level 11!

HelehexOP•16mo ago

what do you mean by this? i've tried doing some of the other stuff you mention

TeamPuzel•16mo ago

Well when you copy the array (and increase the reference count) you solve the problem of a double free. But you still have the problem of shared mutable state, now you have 2 arrays that point to the same memory if you try using both at once, like write something from one array into the other, but they both point to the same storage, you will be mutating an array while also reading from it A copy must mean creating a new array. what you can also do is copy-on-write, meaning you defer this operation to when trying to mutate an array, i.e. before writing to it your accessor should check if the reference count is greater than 1, if so copy This is a danger in reference semantics based languages. The Objective-C array has a warning that you may not under any circumstances read and modify the same array if I recall correctly

HelehexOP•16mo ago

when i do:

var arr = Array[Int](size)
arr = Array[Int](new_size, arr)

@always_inline
fn __init__(inout self, size: Int, owned array: Self): # owned array for the case of infinite self reference (self = array)
    self._size = size
    self._rc = Pointer[Int].alloc(1)
    self._data = Pointer[T].alloc(size) # uses the size defined localy
    self._rc.store(0)
    self.copy(array)
    self.clear(min(size, array._size), size)

@always_inline
fn copy(self, array: Self):
    memcpy(self._data, array._data, min(self._size, array._size))

var arr = Array[Int](size)
arr = Array[Int](new_size, arr)

@always_inline
fn __init__(inout self, size: Int, owned array: Self): # owned array for the case of infinite self reference (self = array)
    self._size = size
    self._rc = Pointer[Int].alloc(1)
    self._data = Pointer[T].alloc(size) # uses the size defined localy
    self._rc.store(0)
    self.copy(array)
    self.clear(min(size, array._size), size)

@always_inline
fn copy(self, array: Self):
    memcpy(self._data, array._data, min(self._size, array._size))

TeamPuzel•16mo ago

I don't think you're using this anywhere Wouldn't you have to use the ^ operator to pass ownership? I don't see you using that operator anywhere

HelehexOP•16mo ago

i tried using that it doesnt do much if the variable isnt used afterwards anyway

TeamPuzel•16mo ago

I think if you don't use it it's going to pass a copy instead of consuming it I would have to check though

HelehexOP•16mo ago

if you had to guess, is the problem with the containers, or the graph?

TeamPuzel•16mo ago

Well you're saying it fails if you use it twice right I would assume that's a memory bug with the array itself, since you're copying it

HelehexOP•16mo ago

also depending on how i use it, it might fail ok

TeamPuzel•16mo ago

Yeah that doesn't seem great you could try checking for out of bounds access just in case too unless you're doing that aready

HelehexOP•16mo ago

oh yeah thats a good point, didnt even htink about that

TeamPuzel•16mo ago

Yeah I've had weird bugs in languages like Zig, and turning on the ReleaseSafe mode (which does bounds checking) would often help me find those I think you can use debug_assert for it, I used it for the Optional

fn unwrap(self) -> T:
    debug_assert(self.is_some(), "Unwrapped a none value")
    return self.raw_value

fn unwrap(self) -> T:
    debug_assert(self.is_some(), "Unwrapped a none value")
    return self.raw_value

it's imported automatically

Mahmoud Abduljawad•16mo ago

This is one interesting case that reminds me of early days of rust becoming mainstream and many users still confused about how ownership and borrow works in it. This conversation here could be elaborated to become a full insightful look at the same with Mojo. Do it, @TeamPuzel.

HelehexOP•16mo ago

does this have anything to do with currently missing features?

TeamPuzel•16mo ago

In a sense. the most primitive structures like smart pointers or arrays are very difficult to get right with an ownership system

HelehexOP•16mo ago

i've made a few already that worked fine

TeamPuzel•16mo ago

Yeah, what I mean is ownership provides guanrantees when you're using things, but it's known to make "unsafe" operations harder And the thing about UB is that it sometimes can work it's hard to know from testing if something is actually correct

HelehexOP•16mo ago

i was originally having the graph own all its containers, but i was running into even more issues there and non mutable, ect..

TeamPuzel•16mo ago

What I'm trying to say is that your container needs to be designed in a way where even though you manually implement it it still respects all the assumptions the compiler is going to make.

HelehexOP•16mo ago

i decided to do it this way bc i figured the ownership features werent ready yet

TeamPuzel•16mo ago

Lifetimes are not, ownership works an example:

from list import List

fn consume(owned list: List[Int]):
    pass

fn main():
    let list: List[Int] = [1, 2, 3]
    consume(list^)
    print(list[0])

from list import List

fn consume(owned list: List[Int]):
    pass

fn main():
    let list: List[Int] = [1, 2, 3]
    consume(list^)
    print(list[0])

the error messages are not very good but this will not compile but that's because the list does not have __copyinit__ I am forced to use ^ here to consume it If you make it copyable you will have to do extra work to make sure no mutable state is shared

HelehexOP•16mo ago

originally i had all move only and cosuming but i couldnt get as far

TeamPuzel•16mo ago

Well I unfortunately don't know what you did that caused problems in that case If you have a link I could try to see You could just try doing something simpler with your array before you try and go this far first make sure everything works individually.

HelehexOP•16mo ago

thats how i started, there is a section where i tested all the arrays features, and everything works. i may go back and try to get some other patterns working like you mention

TeamPuzel•16mo ago

It's a bit early for Mojo to have detailed explanations on this so you can reference Rust's

TeamPuzel•16mo ago

Such as this https://doc.rust-lang.org/nomicon/aliasing.html

Aliasing - The Rustonomicon

The Dark Arts of Advanced and Unsafe Rust Programming

TeamPuzel•16mo ago

There's a lot of explanations why pointers in languages like Rust, Mojo, Swift etc are "unsafe" to use

HelehexOP•16mo ago

yes, unfortunately it's not easy to wrap unsafe code safely rn, but my containers dont have any problems in my tests

TeamPuzel•16mo ago

What are you testing?

HelehexOP•16mo ago

it's features

HelehexOP•16mo ago

oh theres some textual errors there, but the code lines up

TeamPuzel•16mo ago

Do you test if changes to one table affect another? I will copy the code and see if I can find what's happening

fn main():
    var table: Table[Int] = Table[Int](5, 5, 0)
    var table2: Table[Int] = Table[Int](table)
    
    print("table1")
    print(str(table))
    print("table2")
    print(str(table))
    
    table[Ind2(2,5)] = 5
    
    print("table1")
    print(str(table))
    print("table2")
    print(str(table))

fn main():
    var table: Table[Int] = Table[Int](5, 5, 0)
    var table2: Table[Int] = Table[Int](table)
    
    print("table1")
    print(str(table))
    print("table2")
    print(str(table))
    
    table[Ind2(2,5)] = 5
    
    print("table1")
    print(str(table))
    print("table2")
    print(str(table))

This has undefined behavior Sometimes it crashes sometimes not and I'm not seeing changes in the tables it's printing Am I using it wrong?

HelehexOP•16mo ago

works fine for me, but your index is 5, and your length is 5 so your setting out of bounds

TeamPuzel•16mo ago

Oh yeah

HelehexOP•16mo ago

i have a few things imma try i put bounds checks on everything, but it still only shows a problem when run twice. i also move the process out of the graph container, and made it move-only and being consumed for use, thought that might work but ig not it gives an out of bounds on the second run, but not the first is that normal? all the code is inside a function

HelehexOP•16mo ago

i think i've narrowed it down to here but that might just be where the issue is showing itself

HelehexOP•16mo ago

wait why is my lb_to_id() wrong now? it wasnt before 🤔 🧐 im not sure i was accounting for the final pop, but theres still an issue can this be done currently

TeamPuzel•16mo ago

Is there a reason it shouldn't be? I would have to understand what's happening in most functions to say what's wrong :/ It's probably one line of code hiding somewhere 🙂

HelehexOP•16mo ago

because it's a new language

ModularBot•16mo ago

Congrats @Ghostfire The Insatiable, you just advanced to level 6!

HelehexOP•16mo ago

think i got it

TeamPuzel•16mo ago

What was it?

HelehexOP•16mo ago

thanks for some of the help

HelehexOP•16mo ago

had to keep the trace and mask alive long enough

TeamPuzel•16mo ago

oh they were destroyed too early? Didn't you have reference counting? I guess lifetimes aren't working yet so this is more difficult than it has to be

jmky•16mo ago

anyway can you share your codes in a gist or something? using the playground, i am unable to see the codes

HelehexOP•16mo ago

will do

HelehexOP•16mo ago

@jmky https://github.com/helehex/mojo-test

GitHub

GitHub - helehex/mojo-test: Unfolders, arrays, tables and other moj...

Unfolders, arrays, tables and other mojo test examples - GitHub - helehex/mojo-test: Unfolders, arrays, tables and other mojo test examples

jmky•15mo ago

i love your impl of array 2 comments 1. maybe you might want to separate out _rc as a type itself, so it can be reused by others 2. might consider making it threadsafe

Gaming

Programming

Running twice gives error