search

scipopt / russcip

Rust interface for SCIP
https://crates.io/crates/russcip
Apache License 2.0
34 stars 11 forks source link

Possible dangling raw pointers #16

Closed mmghannam closed 1 year ago

mmghannam commented 1 year ago 
          In other methods, you copy the `self.scip` pointer. Do you have a guarantee that you don't have any other references to it anywhere else when you get there ?

A quick look seems to indicate that one could drop the model but keep variables, or a solution, that still contain a pointer to the scip struct.

You should probably be more careful with copying raw pointers. At least, write a comment explaining why you think it's safe every time. At best, just never do it, make a single rust struct for each type of C pointer, and then manage that memory with traditional rust memory management.

_Originally posted by @lovasoa in https://github.com/mmghannam/russcip/pull/13#discussion_r1111264447_

create-issue-branch[bot] commented 1 year ago

Branch issue-16-Possible_dangling_raw_pointers created!

mmghannam commented 1 year ago

Fixed in #20

lovasoa commented 1 year ago

Let's maybe add more tests before marking this as closed ? A quick look at the code doesn't seem like it is really guaranteeing correct memory management. If the current logic is perfectly sound, maybe add some comments to explain what are the invariants and how they are enforced ?

mmghannam commented 1 year ago

Update: Added a memory leak test to CI in #26