HiPhish / cl-cmark

CommonMark implementation for Common Lisp based on libcmark. This system wraps the official reference implementation library, written in C, and provides lispy bindings for Common Lisp.
https://gitlab.com/HiPhish/cl-cmark
BSD 2-Clause "Simplified" License
2 stars 3 forks source link

"It is the caller's responsibility to free the returned buffer." #5

Closed krsanky closed 3 months ago

krsanky commented 3 months ago

Lots of the common lisp doctrsings have the phrase: "It is the caller's responsibility to free the returned buffer."

Which implies that there is something to be done by the user of the library. But with the notions of the type :string in cffi this is handled automatically.

I am not sure of this, but is how I read the cffi docs: https://cffi.common-lisp.dev/manual/cffi-manual.html#Other-Types

Is my understanding correct? Does that make sense that carrying over that string: "It is the caller's responsibility to free the returned buffer." from the c functions is confusing ?


For a specific example, if I want to convert a string to html using markdown-to-html, do I need to do anything with the returned string with respect to freeing the memory?

HiPhish commented 3 months ago

That's a good question. I copied the text over from the man page of libcmark without giving it much thought. The documentation certainly reads as if you are right, at least if the function is called directly via foreign-funcall. Looking at the source of CFFI defcfun expands to a defun which calls foreign-funcall in its body. Is there a way we can test for memory leaks? I tried this little script:

(ql:quickload :libcmark)

(defun derp ()
  (dotimes (_ 1000000)
    (libcmark:render-xml (libcmark:parse-document "Hello *world*." 14 0) 0)))

(sb-profile:profile derp)

(derp)
(sb-profile:report)

The reported result is

measuring PROFILE overhead..done
  seconds  |     gc     |     consed    | calls |  sec/call  |  name  
-----------------------------------------------------------
     4.385 |      0.045 | 1,375,984,592 |     1 |   4.385015 | DERP
-----------------------------------------------------------
     4.385 |      0.045 | 1,375,984,592 |     1 |            | Total

estimated total profiling overhead: 0.00 seconds
overhead estimation parameters:
  3.384e-9s/call, 1.225932e-6s total profiling, 5.77616e-7s internal profiling

However, this does not tell us anything about the amount of memory used. I would expect that if there is a memory leak that the amount of consumed memory would grow, while it would remain more or less constant if the old string was garbage-collected.

krsanky commented 3 months ago

Thanks for investigating!
I am pretty much satisfied at this point I don't need to worry about freeing something with the returned :string values.

I'm using libcmark:markdown-to-html quite nicely in my web server code now, thanks!

I tried different freeing approaches like cffi's foreign-string-tree, but got errors because all you have is a :string at that point. Like I couldn't find any mechanism to free something that was just a string on the lisp side.

"Is there a way we can test for memory leaks?" I don't know, but if I find something I'll report back.