Subject:
[ruby-ffi] Re: Pointer Freeing Bug
From:
JEG2
Date:
1/6/10 2:43 PM
To:
ruby-ffi

On Jan 6, 12:38 pm, Luis Lavena <luislav...@gmail.com> wrote:
> What about forcing the garbage collect process on each spec?

This was a great tip that led me straight to the issue.  Thanks for
the idea.

The problem was a callback I had setup (call() and func() are a
trivial FFI DSL I made):

        call :name    => :TCPDPROC,
             :args    => [:pointer, :int, :pointer, :pointer],
             :returns => :pointer
        func :name    => :putproc,
             :args    =>
[:pointer, :pointer, :int, :pointer, :int, :TCPDPROC,
                          :pointer],
             :returns => :bool

and was using like this:

        callback = lambda { |old_value_pointer, old_size,
returned_size, _|
          old_value   = old_value_pointer.get_bytes(0, old_size)
          replacement = yield(key, old_value, value).to_s
          returned_size.put_int(0, replacement.size)
          FFI::MemoryPointer.from_string(replacement)
        }
        try(:putproc, k, k.size, v, v.size, callback, nil)

The problem seems to be the FFI::MemoryPointer allocation.  Tokyo
Cabinet says in the documentation that it will free the returned
pointer:

/* Store a record into a hash database object with a duplication
handler.
   `hdb' specifies the hash database object connected as a writer.
   `kbuf' specifies the pointer to the region of the key.
   `ksiz' specifies the size of the region of the key.
   `vbuf' specifies the pointer to the region of the value.  `NULL'
means that record addition is
   ommited if there is no corresponding record.
   `vsiz' specifies the size of the region of the value.
   `proc' specifies the pointer to the callback function to process
duplication.  It receives
   four parameters.  The first parameter is the pointer to the region
of the value.  The second
   parameter is the size of the region of the value.  The third
parameter is the pointer to the
   variable into which the size of the region of the return value is
assigned.  The fourth
   parameter is the pointer to the optional opaque object.  It returns
the pointer to the result
   object allocated with `malloc'.  It is released by the caller.  If
it is `NULL', the record is
   not modified.  If it is `(void *)-1', the record is removed.
   `op' specifies an arbitrary pointer to be given as a parameter of
the callback function.  If
   it is not needed, `NULL' can be specified.
   If successful, the return value is true, else, it is false.
   Note that the callback function can not perform any database
operation because the function
   is called in the critical section guarded by the same locks of
database operations. */
bool tchdbputproc(TCHDB *hdb, const void *kbuf, int ksiz, const void
*vbuf, int vsiz,
                  TCPDPROC proc, void *op);

I'm assuming it does free it, and then FFI tries to free the same
already freed pointer.

I was able to "fix" the issue using this code:

        callback = lambda { |old_value_pointer, old_size,
returned_size, _|
          old_value   = old_value_pointer.get_bytes(0, old_size)
          replacement = yield(key, old_value, value).to_s
          returned_size.put_int(0, replacement.size)
          pointer = Utilities.malloc(replacement.size)
          pointer.put_bytes(0, replacement)
          pointer
        }
        try(:putproc, k, k.size, v, v.size, callback, nil)

The Utilities.malloc() call is an FFI function wrapping tcmalloc():

/* Allocate a region on memory.
   `size' specifies the size of the region.
   The return value is the pointer to the allocated region.
   This function handles failure of memory allocation implicitly.
Because the region of the
   return value is allocated with the `malloc' call, it should be
released with the `free' call
   when it is no longer in use. */
void *tcmalloc(size_t size);

It seems to work, but I wasn't sure how to define the size_t
parameter.  I assumed it's an unsigned long and defined it as such:

    func :name    => :malloc,
         :args    => :ulong,
         :returns => :pointer

Can I really count on that though?  Is it portable?

Is there a better way for me to define a pointer that FFI won't try to
free?  I originally tried using FFI::Pointer (instead of
FFI::MemoryPointer), but I couldn't seem to make that work.

Again, thanks for the advice.

James Edward Gray II