investigate whether CFFI frees C-allocated memory for :string return types

[stylewarning]

Tweedledum integration makes use of this, and if it's not, it'll be a slow memory leak.

[stylewarning]

If YES, we should add a comment. If NO, we should fix it.

[appleby]

It appears the memory is not freed by cffi. The cffi docs say

The :string type performs automatic conversion between Lisp and C strings. Note that, in the case of functions the converted C string will have dynamic extent (i.e. it will be automatically freed after the foreign function returns).

which is somewhat ambiguous and makes it seem like maybe the foreign string would be freed, but on further review I think the above only refers to strings passed as arguments to foreign functions which were allocated by cffi as part of the lisp->cstring conversion.

For strings returned from foreign functions, the magic happens in the generic function translate-from-foreign. The :string type ultimately is parsed into a foreign-string-type, which has the following method defined:

(defmethod translate-from-foreign (ptr (type foreign-string-type))
  (unwind-protect
       (values (foreign-string-to-lisp ptr :encoding (fst-encoding type)))
    (when (fst-free-from-foreign-p type)
      (foreign-free ptr))))

You can see the memory is freed only when fst-free-from-foreign-p is true. But by default, it's nil for foreign-string-types.

(define-foreign-type foreign-string-type ()
  (;; CFFI encoding of this string.
   (encoding :initform nil :initarg :encoding :reader encoding)
   ;; Should we free after translating from foreign?
   (free-from-foreign :initarg :free-from-foreign
                      :reader fst-free-from-foreign-p
                      :initform nil :type boolean)
   ;; Should we free after translating to foreign?
   (free-to-foreign :initarg :free-to-foreign
                    :reader fst-free-to-foreign-p
                    :initform t :type boolean))
  (:actual-type :pointer)
  (:simple-parser :string))

This is easy to verify in the REPL

CL-USER> (cffi::parse-type :string)
#<CFFI::FOREIGN-STRING-TYPE :UTF-8>
CL-USER> (cffi::fst-free-from-foreign-p (cffi::parse-type :string))
NIL

Just to be sure, I modified translate-from-foreign to print a debug message and tried calling cl-quil.tweedledum:systhesis-dbs.

TWEEDLEDUM> (in-package :cffi)
#<PACKAGE "CFFI">
CFFI> (defmethod translate-from-foreign (ptr (type foreign-string-type))
	   (unwind-protect
		(values (foreign-string-to-lisp ptr :encoding (fst-encoding type)))
	     (if (fst-free-from-foreign-p type)
		 (progn (format t "freeing string: ~a~%" ptr)
			(foreign-free ptr))
		 (progn (format t "NOT freeing ~a~%" ptr)
			nil))))
WARNING:
   redefining TRANSLATE-FROM-FOREIGN (#<SB-PCL:SYSTEM-CLASS COMMON-LISP:T>
                                      #<STANDARD-CLASS CFFI::FOREIGN-STRING-TYPE>) in DEFMETHOD
#<STANDARD-METHOD CFFI:TRANSLATE-FROM-FOREIGN (T FOREIGN-STRING-TYPE) {100A05B403}>
CFFI> (in-package :cl-quil.tweedledum)
#<PACKAGE "CL-QUIL.TWEEDLEDUM">
TWEEDLEDUM> (synthesis-dbs (list 1 0))
NOT freeing #.(SB-SYS:INT-SAP #X7F374C0080D0)
"H 0
RZ(pi) 0
H 0
"

This actually makes some sense as a default behavior, since cffi can't really know for an arbitrary C function returning char * whether it's safe to call free on the returned pointer.

Possible workarounds

Caveat lector: I haven't tested any of these beyond REPL-poking.

Option 1: tell cffi to :free-from-foreign

Probably the simplest workaround is to switch the return type of the foreign function from :string to (:string :free-from-foreign t).

CL-USER> (in-package :cl-quil.tweedledum)
#<PACKAGE "CL-QUIL.TWEEDLEDUM">
TWEEDLEDUM> (defcfun (%synthesis-dbs "tweedledum_synthesis_dbs")
		(:string :free-from-foreign t)
	      (perm (:pointer :uint32))
	      (size :int))
WARNING: redefining CL-QUIL.TWEEDLEDUM::%SYNTHESIS-DBS in DEFUN
%SYNTHESIS-DBS
TWEEDLEDUM> (synthesis-dbs (list 1 0))
freeing string: #.(SB-SYS:INT-SAP #X7F374C006F30)
"H 0
RZ(pi) 0
H 0
"

Option 2: :string+ptr

Alternatively, switch the return type from :string to :string+ptr, which returns both the converted lisp string and the foreign pointer, so you can then free the foreign pointer yourself. From the cffi docs:

Foreign Type: :string+ptr
Like :string but returns a list with two values when convert from C to Lisp: a Lisp string and the C string’s foreign pointer.

CFFI> (foreign-funcall "getenv" :string "SHELL" :string+ptr)
⇒ ("/bin/bash" #.(SB-SYS:INT-SAP #XBFFFFC6F))

The result would look something like:

TWEEDLEDUM> (defcfun (%synthesis-dbs "tweedledum_synthesis_dbs")
		:string+ptr
	      (perm (:pointer :uint32))
	      (size :int))

(defun synthesis-dbs (permutation)
  (with-foreign-object (perm :uint32 (length permutation))
    (loop :for i :below (length permutation)
          :for p_i :in permutation :do
            (setf (cffi:mem-aref perm :uint32 i) p_i))
    (destructuring-bind (lisp-string foreign-pointer)
        (%synthesis-dbs perm (length permutation))
      (foreign-free foreign-pointer)
      lisp-string)))
WARNING: redefining CL-QUIL.TWEEDLEDUM::%SYNTHESIS-DBS in DEFUN
WARNING: redefining CL-QUIL.TWEEDLEDUM:SYNTHESIS-DBS in DEFUN
SYNTHESIS-DBS
TWEEDLEDUM> (synthesis-dbs (list 1 0))
NOT freeing #.(SB-SYS:INT-SAP #X7F374C020020)
"H 0
RZ(pi) 0
H 0
"

Option 3: Let's get crazy

There are other options, like defining your own foreign-type that derives from foreign-string-type and then doing whatever you want in your custom translate-from-foreign method. But this seems strictly more complicated than the above two options, and with no added benefit.

[stylewarning]

@appleby This is a very great analysis. I think the option to the :string type is best. I didn't know that option existed!

[appleby]

If no one beats me to it, I will send a PR later adding the :free-from-foreign option to the return type.

I think the option to the :string type is best. I didn't know that option existed!

This was my first peek under the hood of cffi, and I was suitably impressed. Defaults are sane but with plenty of knobs for when you need more control. For example, when defining your own foreign types, you can avoid the overhead of method dispatch for the translate-{to,from}-foreign methods by instead defining methods called expand-{to,from}-foreign that return a lisp form to be expanded inline by the defcfun macro. Pretty slick!