Using a defer func() closure to call c.free

[nda...@turnitin.com]

Hi all, 

Apologies if this has already been asked and answered elsewhere, I took a look and couldn't find anything.

I have a simple cgo program that looks a little something like:

func doTheCGo(){
    f := C.CString("/path/to/file.ext")
    o := C.CString("useful values for c call")
    defer func(){
        C.free(unsafe.Pointer(f))
        C.free(unsafe.Pointer(o))
    }()
    // call a C function with f and o; this function does NOT mutate this objects
}    

This results in a double free and then a stack underflow exception.

I have tried the code such that I do defer C.free(unsafe.Pointer(f) immediately after f is created, and the same for o. That works. I have tried freeing right at the end of my function, with no defer, and that works too. 

I understand that the convention is:

f := C.CString("/path/to/file.ext")
defer C.free(unsafe.Pointer(f))

I have now moved the code to that pattern. 

I have tried printing the addresses in a closure and they are the same as they are on creation, and the same as they are immediately prior to return.

What I am wondering is why the closure approach does not work?

Thanks

Nathan

[Jan Mercl]

On Thu, Oct 11, 2018 at 10:15 AM <nda...@turnitin.com> wrote:

> What I am wondering is why the closure approach does not work?

ISTM it should work. Please post a full, standalone reproduction code here or at the issue tracker, thanks.

--

-j

[nda...@turnitin.com]

package main

/*

#cgo CFLAGS: -g -Wall -I/usr/local/include

#cgo LDFLAGS: -L/usr/local/lib -lm -lstdc++

#include <stdlib.h>

#include <string.h>

#include <stdint.h>

#include <math.h>

// tet_pcos_get_string wraps the TET_pcos_get_string so we can use it in Go, cgo does not support

// variadic args, this is the solution

char* str_add(const char *s1, const char *s2) {

   char *result = malloc(strlen(s1) + strlen(s2) + 1); // +1 for the null-terminator

   // in real code you would check for errors in malloc here

   strcpy(result, s1);

   strcat(result, s2);

   return result;

}

*/

import "C"

import (

   "fmt"

   "unsafe"

)

func  main() {

   str := doTheCStuff()

   fmt.Println(str)

}

func doTheCStuff() string{

   s1 := C.CString("string one")

   //defer C.free(unsafe.Pointer(file))

   s2 := C.CString("string 2")

   //defer C.free(unsafe.Pointer(opt))

    defer func(){

       C.free(unsafe.Pointer(s1))

       C.free(unsafe.Pointer(s2))

   }()

   

   s3 := C.GoString(C.str_add(s1, s2))

   return s3

}

This is the sort of thing, this will run just fine and no re-produce the error. The exact use case in production where I see this involves using a 3rd party C library that opens a document. This then, sometimes, results in the error. The trouble is, using the 3rd party lib requires a license key so I'm not sure how much I can post on here.

The actual snippet from prod that occasionally causes this issue is:

// openDoc returns the document handle for the metadata extraction

func (e *Extractor) openDoc(fp string) (*doc, error) {

    file := C.CString(fp)

    defer C.free(unsafe.Pointer(file))

    opt := C.CString("shrug checkglyphlists=true decompose={none} tetml={elements={docinfo=true}} fold={{[U+0640] preserve} {[:Private_Use:] unknownchar} {[:space:] remove} {[U+0009] remove} {[U+000D] remove} {[:Control:] remove} {[:Unassigned:] preserve} {[U+FFFF] remove}} unknownchar=U+003F allowjpeg2000=true")

    defer C.free(unsafe.Pointer(opt))

    

    // For documentation see page 175

    docNum := C.TET_open_document(e.tet, file, 0, opt)

    if docNum < 0 {

        return nil, fmt.Errorf("failed to open document")

    }

    doc := &doc{

        num: docNum,

        tet: e.tet, // added for convenience

    }

    return doc, nil

}

For this to work you need to have an instance ofTET and a license key. I can't share that, for obvious reason. But What you can see in this code is that I now have the more common pattern for create and free. Yet when I use the defer closure pattern here, after creating both file and opt, I sometimes get the double free. We have have noticed that the issue was reproducible with a specific set of files. Using this pattern fixes that issue, I'm just curious as to why the defer would cause trouble.

[Robert Engels]

Are you sure the document is not doing the free of the file and the opt when it is destroyed? It might need those references beyond the function scope. 

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[nda...@turnitin.com]

Hi, 

That was at least a working theory there for a while; that perhaps the open document was doing some freeing of the memory before it returned. The reason we discounted that was some docs the defer func() free works fine. It could be a weird bug in that library, and I'm still considering that option :) What moved us away from that is that when we had the defer func() approach a certain set of docs failed every time with this issue. Move to the code we have now and all those docs work as expected.

It's a really odd one. I'm ok with the code as it is now. I just would like to understand some more. Thanks for taking a look and for asking questions. Please keep doing so, you never know we may get to the bottom of this.

Thanks

Nathan

[Robert Engels]

It is not necessarily the open doc that is freeing the memory, could be the close doc as well - meaning it needs the file and opt reference while it is open. Probably not file, but possible opt. The other thing are you sure that the opt doesn’t contain a reference that is freed to early...

[Nathan Davies]

Ok, that's an interesting line of investigation - I'll take a look into that and let you know if I find anything.

Thanks

Nathan

--

Nathan Davies

Senior Software Engineer

Turnitin

36 Gallowgate, Wellbar Central, Newcastle upon Tyne, NE1 4TD, UK

Direct: +44 (0) 191 681-0334 (internally Ext: 2344)

Office: +44 (0) 191 681-0200

Email: nda...@turnitin.com

Web: www.turnitin.com

Turnitin is a Registered product of Turnitin UK, Ltd which is a wholly owned subsidiary of Turnitin, LLC. Turnitin UK, Ltd is a company registered in England and Wales with company number   07321841.                                                  

DISCLAIMER: This e-mail and any files transmitted with it are confidential and intended solely for use by the recipient to which it is addressed. If this email has been misdirected in error please contact us on    +44 (0) 191 681 0200. This e-mail and any attachments have been scanned for viruses prior to sending. No liability will be accepted for any loss incurred as a result of any viruses being passed on. Any form of unauthorised publication or distribution is strictly prohibited.