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, &idx)' 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 (&image),
> >              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 +0000, 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_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(&context);
> > >         aut->ctx.device = NULL;
> > >         aut->ctx.gmloop = NULL;
> > >     }
> > > 
> > >     g_clear_object(&aut->ctx.clops); // Free GCancellable.
> > > 
> > >     return NULL;
> > > }
> > > //---------------------------------------------------------------
> > > --
> > > --
> > > --------
> > > 
> > > static void etr_fp_extract_minutiae(FpImage *img, GAsyncResult *r
> > > es
> > > , 
> > > 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,&error))
> > >     {
> > >         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 *re
> > > s,
> > >  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, &error);
> > > 
> > >     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;
> > > }
> > > //---------------------------------------------------------------
> > > --
> > > --
> > > --------
> > > 
> > > void capturing__entry(void* aut_data)
> > > {
> > >     g_idle_add(etr_fp_start_capture,aut_data);
> > > }
> > > //---------------------------------------------------------------
> > > --
> > > --
> > > --------
> > > 
> > > static gboolean etr_fp_dev_close(gpointer data)
> > > {
> > >     g_autoptr(GError) gerror = NULL;
> > >     tAUTOMATON_DATA *aut = (tAUTOMATON_DATA *)data;
> > > 
> > >     if (!fp_device_close_sync(aut->ctx.device,NULL,&gerror))
> > >         LogError("Error closing device: %s",gerror->message);
> > > 
> > >     g_main_loop_quit(aut->ctx.gmloop);
> > > 
> > >     return FALSE;
> > > }
> > > //---------------------------------------------------------------
> > > --
> > > --
> > > --------
> > > 
> > > int etr_fp_init(void *aut)
> > > {
> > >     /*
> > >       do other things.
> > >     */
> > > 
> > >     if (pthread_create(&aut-
> > > > ctx.event_t,NULL,&etr_fp_gmain_loop,aut))
> > >     {
> > >         LogError("%s:Id%d Unable to create glib event loop thread
> > > ",
> > > K_
> > > ETR_FP_API_PREFIX,id);
> > >         return K_ETR_FP_FAILURE;
> > >     }
> > > 
> > >     return K_ETR_FP_OK;
> > > }
> > > //---------------------------------------------------------------
> > > --
> > > --
> > > --------
> > > 
> > > int etr_fp_end(void *aut)
> > > {
> > >     // Free libfrpint, glib resources when gmainloop ends free au
> > > to
> > > ma
> > > ton.;
> > >     g_idle_add_full(G_PRIORITY_HIGH_IDLE,etr_fp_dev_close,aut,NUL
> > > L)
> > > ;
> > >     pthread_join(aut->ctx.event_t,NULL);
> > > 
> > >     return 0;
> > > }
> > > //---------------------------------------------------------------
> > > --
> > > --
> > > --------
> > > 
> > >  
> > >  
> > > I have read some things about valgrind with Glib and possible
> > > problems. So,my first question is:
> > >  
> > > Can I use Valgrind with libfprint?
> > >  
> > > When I run valgrind with the following configuration:
> > >  
> > > valgrind --leak-check=full --show-leak-kinds=definite,indirect --
> > > track-origins=yes --verbose --log-file=valgrind-out.txt ./myapp
> > >  
> > > The following catches my attention:
> > >  
> > > ==35461== 1,113,600 bytes in 10 blocks are indirectly lost in
> > > loss
> > > record 910 of 912
> > > ==35461==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-
> > > gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> > > ==35461==    by 0x4AECCB8: g_malloc (in /usr/lib/x86_64-linux-
> > > gnu/libglib-2.0.so.0.6400.6)
> > > ==35461==    by 0x5450990: binarize_image_V2 (in /usr/lib/x86_64-
> > > linux-gnu/libfprint.so.2.0.0)
> > > ==35461==    by 0x5450B3C: binarize_V2 (in /usr/lib/x86_64-linux-
> > > gnu/libfprint.so.2.0.0)
> > > ==35461==    by 0x544605A: lfs_detect_minutiae_V2 (in
> > > /usr/lib/x86_64-linux-gnu/libfprint.so.2.0.0)
> > > ==35461==    by 0x543EE8D: get_minutiae (in /usr/lib/x86_64-
> > > linux-
> > > gnu/libfprint.so.2.0.0)
> > > ==35461==    by 0x540E7C1: fp_image_detect_minutiae_thread_func
> > > (in
> > > /usr/lib/x86_64-linux-gnu/libfprint.so.2.0.0)
> > > ==35461==    by 0x8457C81: ??? (in /usr/lib/x86_64-linux-
> > > gnu/libgio-
> > > 2.0.so.0.6400.6)
> > > ==35461==    by 0x4B111B3: ??? (in /usr/lib/x86_64-linux-
> > > gnu/libglib-
> > > 2.0.so.0.6400.6)
> > > ==35461==    by 0x4B10910: ??? (in /usr/lib/x86_64-linux-
> > > gnu/libglib-
> > > 2.0.so.0.6400.6)
> > > ==35461==    by 0x858C608: start_thread (pthread_create.c:477)
> > > ==35461==    by 0x4CE2292: clone (clone.S:95)
> > > ==35461== 
> > > ==35461== 1,113,600 bytes in 10 blocks are indirectly lost in
> > > loss
> > > record 911 of 912
> > > ==35461==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-
> > > gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> > > ==35461==    by 0x4AECCB8: g_malloc (in /usr/lib/x86_64-linux-
> > > gnu/libglib-2.0.so.0.6400.6)
> > > ==35461==    by 0x540EF98: fp_image_detect_minutiae (in
> > > /usr/lib/x86_64-linux-gnu/libfprint.so.2.0.0)
> > > ==35461==    by 0x484E10B: etr_fp_dev_capture_cb
> > > (fingerprint.c:528)
> > > ==35461==    by 0x8456F68: ??? (in /usr/lib/x86_64-linux-
> > > gnu/libgio-
> > > 2.0.so.0.6400.6)
> > > ==35461==    by 0x8457B5C: ??? (in /usr/lib/x86_64-linux-
> > > gnu/libgio-
> > > 2.0.so.0.6400.6)
> > > ==35461==    by 0x54375AA: fp_device_task_return_in_idle_cb (in
> > > /usr/lib/x86_64-linux-gnu/libfprint.so.2.0.0)
> > > ==35461==    by 0x4AE6E6D: g_main_context_dispatch (in
> > > /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.6400.6)
> > > ==35461==    by 0x4AE721F: ??? (in /usr/lib/x86_64-linux-
> > > gnu/libglib-
> > > 2.0.so.0.6400.6)
> > > ==35461==    by 0x4AE7512: g_main_loop_run (in /usr/lib/x86_64-
> > > linux-
> > > gnu/libglib-2.0.so.0.6400.6)
> > > ==35461==    by 0x484DAA7: etr_fp_gmain_loop (fingerprint.c:401)
> > > ==35461==    by 0x858C608: start_thread (pthread_create.c:477)
> > > ==35461== 
> > > ==35461== 2,333,120 (800 direct, 2,332,320 indirect) bytes in 10
> > > blocks are definitely lost in loss record 912 of 912
> > > ==35461==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-
> > > gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> > > ==35461==    by 0x4AECCB8: g_malloc (in /usr/lib/x86_64-linux-
> > > gnu/libglib-2.0.so.0.6400.6)
> > > ==35461==    by 0x4B052A5: g_slice_alloc (in /usr/lib/x86_64-
> > > linux-
> > > gnu/libglib-2.0.so.0.6400.6)
> > > ==35461==    by 0x4B058CD: g_slice_alloc0 (in /usr/lib/x86_64-
> > > linux-
> > > gnu/libglib-2.0.so.0.6400.6)
> > > ==35461==    by 0x837B0CF: g_type_create_instance (in
> > > /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.6400.6)
> > > ==35461==    by 0x835A34C: ??? (in /usr/lib/x86_64-linux-
> > > gnu/libgobject-2.0.so.0.6400.6)
> > > ==35461==    by 0x835C377: g_object_new_valist (in
> > > /usr/lib/x86_64-
> > > linux-gnu/libgobject-2.0.so.0.6400.6)
> > > ==35461==    by 0x835C6CC: g_object_new (in /usr/lib/x86_64-
> > > linux-
> > > gnu/libgobject-2.0.so.0.6400.6)
> > > ==35461==    by 0x5428644: imaging_run_state (in /usr/lib/x86_64-
> > > linux-gnu/libfprint.so.2.0.0)
> > > ==35461==    by 0x543E345: transfer_finish_cb (in
> > > /usr/lib/x86_64-
> > > linux-gnu/libfprint.so.2.0.0)
> > > ==35461==    by 0x8456F68: ??? (in /usr/lib/x86_64-linux-
> > > gnu/libgio-
> > > 2.0.so.0.6400.6)
> > > ==35461==    by 0x8456FAC: ??? (in /usr/lib/x86_64-linux-
> > > gnu/libgio-
> > > 2.0.so.0.6400.6)
> > > ==35461== 
> > > ==35461== LEAK SUMMARY:
> > > ==35461==    definitely lost: 1,128 bytes in 22 blocks
> > > ==35461==    indirectly lost: 2,334,793 bytes in 2,604 blocks
> > > ==35461==      possibly lost: 34,119 bytes in 142 blocks
> > > ==35461==    still reachable: 190,826 bytes in 1,292 blocks
> > > ==35461==                       of which reachable via heuristic:
> > > ==35461==                         length64          : 1,160 bytes
> > > in
> > > 29 blocks
> > > ==35461==                         newarray          : 1,584 bytes
> > > in
> > > 19 blocks
> > > ==35461==         suppressed: 0 bytes in 0 blocks
> > > ==35461== Reachable blocks (those to which a pointer was found)
> > > are
> > > not shown.
> > > ==35461== To see them, rerun with: --leak-check=full --show-leak-
> > > kinds=all
> > > ==35461== 
> > > ==35461== ERROR SUMMARY: 135 errors from 135 contexts
> > > (suppressed:
> > > 0
> > > from 0)
> > >  
> > > So fp_image_detect_minutiae have a memory leak?
> > >  
> > > Or maybe is a Valgrind problem. Every time I run a capture the
> > > memory
> > > usage grows and it doesn't seem to return the resources used.
> > > I am executing the g_main_loop_run in another thread and the
> > > calls
> > > to
> > > the capture function are made usingg_idle_add
> > >  
> > > Another problem detected is when unplug device, next call capture
> > > process. The removed device error is detected correctly but when
> > > I
> > > reinitialize the library creating a new object of type:
> > > 
> > >  * FPContext
> > >  * GmainLoop
> > > 
> > > 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, &idx)' failed
> > > 
> > > 
> > > The library works fine when the device is reconnected, but I want
> > > to
> > > know about these 2 errors.
> > > Thanks for all the support provided.
> > >  
> > >  
> > > _______________________________________________
> > > fprint mailing list
> > > fprint@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/fprint
> >  
> >  
> 
> _______________________________________________
> fprint mailing list
> fprint@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/fprint

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
fprint mailing list
fprint@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/fprint

Reply via email to