triangle icon indicating copy to clipboard operation
triangle copied to clipboard

Avoid writing Go pointers to non Go memory

Open hypirion opened this issue 3 years ago • 2 comments

Hi, thank you for this wrapper! I've been using it on and off for a while, and for me it seems to work as intended for the most part. However, in some situations, I ended up with segmentation faults without any good explanation why.

Yesterday I decided to take a look by using cgo and gc debug flags, and found out that this wrapper passes in Go pointers to C. Passing Go pointers to C isn't allowed because Go's garbage collector may kick in at any time. I couldn't see any other backwards compatible way to do this except copying the slice contents into C memory.

I also did a best effort attempt to clean up C memory after use, although I am not aware of how all the options work. At least one of the pointers – holelist - were copied to the out struct, and that caused issues with double freeing. I wouldn't be surprised if this could be a problem with other allocated memory blocks. There's no double free with the default helper functions, but I guess other flags/options could trigger return values that would be double free'd. This would be a lot simpler if the functions calling triangle either gathered all input/output in a single struct, or did not expose cleanup. However, that would break backwards compatibility.

Finally, Go complained so much about no Go module file that I added an empty one – hopefully that's fine.

hypirion avatar Feb 08 '22 10:02 hypirion

Hi @hypirion, glad you found the wrapper useful. Thanks for the fix and the unit test! Your changes/additions look very sensible to me. I haven't been using Go for a while now, so I have to set things up to test this out. I will merge your PR sometime this week when I get some time.

pradeep-pyro avatar Feb 14 '22 11:02 pradeep-pyro

Hi @pradeep-pyro, we are getting something the same as @hypirion described. Are you planning to merge the fix?

l1va avatar Apr 02 '23 18:04 l1va