From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id gC4ODNfGa2OijAAAbAwnHQ (envelope-from ) for ; Wed, 09 Nov 2022 16:27:19 +0100 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id YLjqC9fGa2O8KAAAauVa8A (envelope-from ) for ; Wed, 09 Nov 2022 16:27:19 +0100 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 948CF23132 for ; Wed, 9 Nov 2022 16:27:18 +0100 (CET) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1osmyn-0003Yz-Ct; Wed, 09 Nov 2022 10:27:05 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1osmyl-0003NF-B3 for bug-guix@gnu.org; Wed, 09 Nov 2022 10:27:03 -0500 Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1osmyl-00070C-0w for bug-guix@gnu.org; Wed, 09 Nov 2022 10:27:03 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1osmyk-0008Sj-EW for bug-guix@gnu.org; Wed, 09 Nov 2022 10:27:02 -0500 X-Loop: help-debbugs@gnu.org Subject: bug#58732: installer: finalizers & device destroy segfault Resent-From: Mathieu Othacehe Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Wed, 09 Nov 2022 15:27:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 58732 X-GNU-PR-Package: guix X-GNU-PR-Keywords: To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 58732@debbugs.gnu.org Received: via spool by 58732-submit@debbugs.gnu.org id=B58732.166800757232469 (code B ref 58732); Wed, 09 Nov 2022 15:27:02 +0000 Received: (at 58732) by debbugs.gnu.org; 9 Nov 2022 15:26:12 +0000 Received: from localhost ([127.0.0.1]:40611 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1osmxv-0008Rd-Of for submit@debbugs.gnu.org; Wed, 09 Nov 2022 10:26:12 -0500 Received: from eggs.gnu.org ([209.51.188.92]:54686) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1osmxq-0008Qw-Gg for 58732@debbugs.gnu.org; Wed, 09 Nov 2022 10:26:10 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1osmxk-0006i3-Gh; Wed, 09 Nov 2022 10:26:00 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:In-Reply-To:Date:References:Subject:To: From; bh=Kg1fGEDllj99l03DT7w+M/FHwksQJXZV/Lyy1+F84aA=; b=JzXCjMgxLfFTuAvF/aB7 Qq0VhmYCqPpTrvB2QuhdUIKXnAhnhqUulHRm20/68yF8utMjin19VJXSA+bDgIRAnpI5eAZbFUNJ+ BRYPF3iqx5qbapjztPekmLNi+5KKtXVvHTh4XI+Nef7wwxx9MGf6mjdgzlBeNeYaxA+YUeIrUGcr/ KAjrHmLs7OAXphSVVf00kqr/rDtIDMEfwQlj6PKL/T9AIsyaEAx3fxYiPCDlskoCLxV/+jIe7ar1e 8COEwfy4oCTwpxJAF8UbBWTRDgwOBbCOpsLRN3ErxNMYi9upckdltCgf/Cew3Se5RiwumBZBZqnJ4 Gi/sp1ADiUuZsg==; Received: from 2a02-8429-81d2-3d01-94c9-8097-ea5c-2775.rev.sfr.net ([2a02:8429:81d2:3d01:94c9:8097:ea5c:2775] helo=meije) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1osmxi-0004V9-9h; Wed, 09 Nov 2022 10:25:58 -0500 From: Mathieu Othacehe References: <87v8oa29ik.fsf@gnu.org> <871qqlwrpy.fsf@gnu.org> <87iljwuwf7.fsf@gnu.org> <871qqkth3g.fsf@gnu.org> <87sfiwm297.fsf@gnu.org> <87iljquc3a.fsf@gnu.org> <8735auwwjf.fsf@gnu.org> Date: Wed, 09 Nov 2022 16:25:55 +0100 In-Reply-To: <8735auwwjf.fsf@gnu.org> (Mathieu Othacehe's message of "Mon, 07 Nov 2022 17:37:24 +0100") Message-ID: <87tu38m9oc.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-guix@gnu.org List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+larch=yhetil.org@gnu.org Sender: bug-guix-bounces+larch=yhetil.org@gnu.org X-Migadu-Flow: FLOW_IN X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1668007638; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:resent-cc:resent-from:resent-sender: resent-message-id:in-reply-to:in-reply-to:references:references: list-id:list-help:list-unsubscribe:list-subscribe:list-post: dkim-signature; bh=Kg1fGEDllj99l03DT7w+M/FHwksQJXZV/Lyy1+F84aA=; b=OUFmb1zp7fBu8KbOc0cf4vsreQu03vc1HgddMl23Hhq6+LVk5+Ss6/NQzhdQsgyLSBhlhC lcZ/vCpxFLZrEH+a+QqTbxQwTi0QSQxOM4BJ/3sgss6uKdlgNCzFSqH+WLCVorjggCyAwK xExzLTHZOQKZ5ILNCV6Ze2mQIcyhdsoXuCwhp3DpO1jTM0VmtHRS09h9QMFdk3JXWYFerx bSwmLa+PVPD6w88ss7VWYKSFosxvWd/FY1tyK2WRupnJUvoY5e4WiKaCw4RAdh01uVQ94l XsMtQo2cbZNRvTipNxTTM+XLQXoABc3Etfvl7nLm0zHgH4DvfmzomeVRgWCyOg== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1668007638; a=rsa-sha256; cv=none; b=nVt/Ek5z5Ja608CSnAV91daD9AOJ2TDNrU4eXvs+wNtoWpnyLVenqg9+VcfLdSyQdRG06m bYn6tTWG5zUKCDDCxT5onPO7JrJ9Q3p8V3A4dAAn1r1NeZ6ezcXx/rC9P5qjJedeHzZyLa qxS/6I91r+aAxlRNEmreuqPwCHQ0BQumnnC2pHZ3QxY55FWTT+Ie6GFugtjeX5b84dWtw3 ywonIK5ZEU4AGLWC056ICv7esi3PSSqjjkr3VQpelnp0tbxDWtIEes9o4kQrzo59ZtG1eX sHwyxOd6uEpVhIoc/CrQkQyNEwxgDEDBlgtyi666wna0XA+KbsW6UbEoj6crbg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gnu.org header.s=fencepost-gnu-org header.b=JzXCjMgx; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of "bug-guix-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="bug-guix-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: -3.92 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gnu.org header.s=fencepost-gnu-org header.b=JzXCjMgx; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of "bug-guix-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="bug-guix-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 948CF23132 X-Spam-Score: -3.92 X-Migadu-Scanner: scn1.migadu.com X-TUID: GIil8edRTeGA --=-=-= Content-Type: text/plain Hey, I ran further tests and my understanding is that the weak hash-table / finalizer mechanism is not compatible with a C function that can return multiple times the same allocated object. Even if we were to introduce a set-pointer-unique-finalizer! procedure that calls scm_i_set_finalizer instead of scm_i_add_finalizer we would still have double free errors because the finalizers are registered on SCM pointers and not on libparted C pointers when calling GC_REGISTER_FINALIZER_NO_ORDER. I tested it out and I had several SCM pointers encapsulating the same libparted C pointer, thus multiple finalizers on the same underlying C pointer. Anyway, here is a patch that solves the issue by removing the device finalizer. It also means that all devices are persisted until the end of the program which doesn't feel right, but I cannot think of a better solution. Let me know if you agree with my reasoning :) Thanks, Mathieu --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Remove-the-finalizer-on-device-pointers.patch >From 066220a75c020b818aab9c2f5c3a7db835fa871a Mon Sep 17 00:00:00 2001 From: Mathieu Othacehe Date: Wed, 9 Nov 2022 16:12:52 +0100 Subject: [PATCH 1/1] Remove the finalizer on device pointers. Fixes: * parted/device.scm (%device-destroy): Remove it. (pointer->device!): Do not set a finalizer. --- parted/device.scm | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/parted/device.scm b/parted/device.scm index 56a774b..be7f0ac 100644 --- a/parted/device.scm +++ b/parted/device.scm @@ -43,20 +43,23 @@ device-get-minimum-alignment device-get-optimum-alignment)) -;; Record all devices, so that pointer finalizers are only set once, -;; even if get-device returns an already known pointer. Use the -;; pointer as key and the associated as value. -(define %devices (make-weak-value-hash-table)) - -(define %device-destroy - (libparted->pointer "ped_device_destroy")) - +;; Record all devices, so that we do not end up with different +;; objects aliasing the same underlying C pointer. Use the pointer as key and +;; the associated as value. +(define %devices (make-hash-table)) + +;; %DEVICES was a weak hash-table and we used to set a finalizer on POINTER. +;; This is inevitably causing double free issues for the following reason: +;; +;; When goes out of scope and is removed from the %DEVICES table, the +;; finalizer that is set on the underlying C pointer is still registered but +;; possibly not called as finalization happens is a separate thread. If a +;; subsequent call to ped_device_get returns the same C pointer, another +;; finalizer will be registered. This means that the finalization function +;; can be called twice on the same pointer, causing a double free issue. (define (pointer->device! pointer) - ;; Check if a finalizer is already registered for this pointer. (or (hash-ref %devices pointer) (let ((device (pointer->device pointer))) - ;; Contrary to its name, this "adds" a finalizer. - (set-pointer-finalizer! pointer %device-destroy) (hash-set! %devices pointer device) device))) -- 2.38.0 --=-=-=--