Re: [fprint] Memory management Libfprint
On Thu, 2021-04-22 at 11:38 +0200, Benjamin Berg wrote:
> I'll submit a fix later, but just keeping the FpContext around (or
> unref'ing it only after the "device-removed" signal has happened)
> should work around the problem.
Or just doing
while (g_main_context_iteration (NULL, FALSE)) {};
before destroying the FpContext.
Benjamin
> > Have the next error:
> >
> > (process:35): libfprint-context-DEBUG: 17:47:21.078: No driver
> > found
> > for USB device 1D6B:0001 ---> Device was removed and not plugged
> > in
> > yet .
> > (process:35): libfprint-image_device-DEBUG: 17:47:21.225: Image
> > device open completed --> Device plugged in again, free up memory
> > and
> > create new objects.
> > (process:35): libfprint-device-DEBUG: 17:47:21.225: Device reported
> > open completion
> > (process:35): libfprint-device-DEBUG: 17:47:21.225: Completing
> > action
> > FPI_DEVICE_ACTION_OPEN in idle!
> > (process:35): GLib-CRITICAL **: 17:47:21.226:
> > g_ptr_array_find_with_equal_func: assertion 'haystack != NULL'
> > failed
> > (process:35): libfprint-context-CRITICAL **: 17:47:21.226:
> > remove_device_idle_cb: assertion 'g_ptr_array_find (priv->devices,
> > data->device, )' failed
> >
>
> Right, that is a bug in FpContext and it is a serious use-after free
> issue. First, it should be using g_signal_connect_object in
> device_removed_cb and async_device_init_done_cb.
>
> But, I suspect in this case it is the idle handler that is added in
> remove_device firing after the FpContext was destroyed. Both of these
> are bugs that are easy to fix.
>
> Benjamin
>
> >
> > The library works fine when the device is reconnected, but I want
> > to
> > know about these 2 errors
> >
> >
> > Do you know what happens?
> >
> > And a final question related to g_idle_add
> >
> > When g_cancellable_cancel is invoked, it needs to be invoked with
> > g_idle_add.
> >
> > The g_main_loop_run is running on another thread.
> >
> > Thanks for your support.
> >
> > From: Benjamin Berg
> > Sent: Wednesday, April 21, 2021 3:30 AM
> > To: Carlos Garcia; fprint@lists.freedesktop.org
> > Subject: Re: [fprint] Memory management Libfprint
> >
> > Hi Carlos,
> >
> > the image returned by fp_device_capture_finish is owned by your
> > code
> > (marked as "transfer full"). It is a GObject, and you are
> > responsible
> > to eventually call g_object_unref on it.
> >
> > There are various ways of achieving that. If you can use the modern
> > GLib autoptr features, then I would suggest:
> >
> > * Change the declaration in etr_fp_dev_capture_cb to:
> > g_autoptr(FpImage) image = NULL;
> > * Change the fp_image_detect_minutiae call to steal the reference:
> > fp_image_detect_minutiae(g_steal_pointer (),
> > aut->ctx.clops,
> > (GAsyncReadyCallback)etr_fp_extract_minutiae,aut);
> > * Add a variable auto unref in etr_fp_extract_minutiae:
> > g_autoptr(FpImage) img_free = img;
> >
> > After that the memory leak should be gone.
> >
> > You can of course also call g_object_unref manually, but then you
> > need
> > to make sure to handle all the error paths correctly.
> >
> > Benjamin
> >
> > On Tue, 2021-04-20 at 23:09 +, Carlos Garcia wrote:
> > >
> > > Hi everyone. I have some questions with the memory management
> > > with
> > > libfprint.
> > > I’m creating a program for capture image and retrieve the
> > > minutiae
> > > from the captured image. I’m using:
> > >
> > > * fp_device_capture
> > > * fp_device_capture_finish
> > > * fp_image_detect_minutiae
> > > * fp_image_detect_minutiae_finish
> > >
> > > I have relied on the img-capture.c example from the version
> > > 1.90.7
> > > examples.
> > > Here is my code:
> > >
> > > ///< This function is executed in another thread using pthreads
> > > static void * etr_fp_gmain_loop(void *data)
> > > {
> > > FpContext *context = NULL;
> > > tAUTOMATON_DATA *aut = (tAUTOMATON_DATA *) data;
> > > aut->ctx.clops = g_cancellable_new();
> > >
> > > // Create libfprint context
> > >
> > > if ((context = etr_fp_find_device(aut)) != NULL) // Inside th
> > > is
> > > f
> > > unction open device with fp_de
Re: [fprint] Memory management Libfprint
Hi Carlos,
the image returned by fp_device_capture_finish is owned by your code
(marked as "transfer full"). It is a GObject, and you are responsible
to eventually call g_object_unref on it.
There are various ways of achieving that. If you can use the modern
GLib autoptr features, then I would suggest:
* Change the declaration in etr_fp_dev_capture_cb to:
g_autoptr(FpImage) image = NULL;
* Change the fp_image_detect_minutiae call to steal the reference:
fp_image_detect_minutiae(g_steal_pointer (),
aut->ctx.clops,
(GAsyncReadyCallback)etr_fp_extract_minutiae,aut);
* Add a variable auto unref in etr_fp_extract_minutiae:
g_autoptr(FpImage) img_free = img;
After that the memory leak should be gone.
You can of course also call g_object_unref manually, but then you need
to make sure to handle all the error paths correctly.
Benjamin
On Tue, 2021-04-20 at 23:09 +, Carlos Garcia wrote:
>
> Hi everyone. I have some questions with the memory management with
> libfprint.
> I’m creating a program for capture image and retrieve the minutiae
> from the captured image. I’m using:
>
> * fp_device_capture
> * fp_device_capture_finish
> * fp_image_detect_minutiae
> * fp_image_detect_minutiae_finish
>
> I have relied on the img-capture.c example from the version 1.90.7
> examples.
> Here is my code:
>
> ///< This function is executed in another thread using pthreads
> static void * etr_fp_gmain_loop(void *data)
> {
> FpContext *context = NULL;
> tAUTOMATON_DATA *aut = (tAUTOMATON_DATA *) data;
> aut->ctx.clops = g_cancellable_new();
>
> // Create libfprint context
>
> if ((context = etr_fp_find_device(aut)) != NULL) // Inside this f
> unction open device with fp_device_open_sync
> {
> KernelInsertEvent(aut->fsm->aut_id,AUT_EVT_OPENED,NULL,0);
> aut->ctx.gmloop = g_main_loop_new(NULL,FALSE);
> g_main_loop_run(aut-
> >ctx.gmloop);// Run until g_main_loop_quit is called.
>
> // Free resources.
> g_main_loop_unref(aut->ctx.gmloop);
> g_clear_object();
> aut->ctx.device = NULL;
> aut->ctx.gmloop = NULL;
> }
>
> g_clear_object(>ctx.clops); // Free GCancellable.
>
> return NULL;
> }
> //---
>
>
> static void etr_fp_extract_minutiae(FpImage *img, GAsyncResult *res,
> void *user_data)
> {
> GPtrArray *minutiaes= NULL;
> g_autoptr(GError) error = NULL;
> tAUTOMATON_DATA *aut= (tAUTOMATON_DATA *) user_data;
>
> if (fp_image_detect_minutiae_finish(img,res,))
> {
> minutiaes = fp_image_get_minutiae(img);
> if (minutiaes != NULL)
> {
> gint img_w = fp_image_get_width(img);
> gint img_h = fp_image_get_height(img);
> // Do something
>
> }
> else
> LogError("Error retrieving minutiae");
> }
> else
> {
> LogError("Error retrieving minutiae from FingerPrint. %s",(er
> ror) ? error->message : "Unknown Error");
> if (error != NULL && error->code == G_IO_ERROR_CANCELLED)
> g_cancellable_reset(aut->ctx.clops);
> }
>
> KernelInsertEvent(aut->fsm->aut_id,AUT_EVT_DATA,NULL,0);
> }
>
> //---
>
>
> static void etr_fp_dev_capture_cb(FpDevice *dev, GAsyncResult *res, v
> oid *user_data)
> {
> int result;
> FpImage *image = NULL;
> g_autoptr(GError) error = NULL;
> tAUTOMATON_DATA *aut = (tAUTOMATON_DATA *) user_data;
>
> image = fp_device_capture_finish(dev, res, );
>
> if (!image)
> {
> LogError("Error capturing fingerprint: %s",error->message);
>
> if (aut->ctx.cap_cb)
> aut->ctx.cap_cb(error->code,NULL,aut->ctx.u_data);
>
> if (error->code == G_IO_ERROR_CANCELLED) {
> g_cancellable_reset(aut->ctx.clops);
> KernelInsertEvent(aut->fsm->aut_id,AUT_EVT_DATA,NULL,0);
> }
>
> else {
>
> etr_fp_dev_close(aut);
> KernelInsertEvent(aut->fsm-
> >aut_id,AUT_EVT_REMOVED,NULL,0);
> }
>
> return;
> }
>
> if (aut->ctx.imgpath != NULL)
> if ((result = etr_fp_save_image_to_pgm(image, aut-
> >ctx.imgpath)) < 0)
> LogError("Unable to save the image in specified path: %s.
> Error code: %d",aut->ctx.imgpath,result);
>
> fp_image_detect_minutiae(image,aut-
> >ctx.clops,(GAsyncReadyCallback)etr_fp_extract_minutiae,aut);
> }
> //---
>
>
> gboolean etr_fp_start_capture(gpointer data)
> {
> tAUTOMATON_DATA *aut = (tAUTOMATON_DATA *) data;
>
> fp_device_capture(aut->ctx.device, TRUE, aut-
> >ctx.clops, (GAsyncReadyCallback) etr_fp_dev_capture_cb, aut);
>
> return FALSE;
> }
>
