All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Edwin Török" <edvin.torok@citrix.com>
To: <xen-devel@lists.xenproject.org>
Cc: "Edwin Török" <edvin.torok@citrix.com>,
	"Christian Lindig" <christian.lindig@citrix.com>,
	"David Scott" <dave@recoil.org>, "Wei Liu" <wl@xen.org>,
	"Anthony PERARD" <anthony.perard@citrix.com>
Subject: [PATCH v1 7/7] tools/ocaml/libs/eventchn: do not leak event channels and OCaml 5.0 compat
Date: Fri, 29 Jul 2022 18:53:30 +0100	[thread overview]
Message-ID: <6e5fd9edfea379b69682fa538141298fc1bc3110.1659116941.git.edvin.torok@citrix.com> (raw)
In-Reply-To: <cover.1659116941.git.edvin.torok@citrix.com>

Add a finalizer on the event channel value, so that it calls
`xenevtchn_close` when the value would be GCed.

In practice oxenstored seems to be the only user of this,
and it creates a single global event channel only,
but freeing this could still be useful when run with OCAMLRUNPARAM=c

The code was previously casting a C pointer to an OCaml value,
which should be avoided: OCaml 5.0 won't support it.
(all "naked" C pointers must be wrapped inside an OCaml value,
 either an Abstract tag, or Nativeint, see the manual
 https://ocaml.org/manual/intfc.html#ss:c-outside-head)

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 tools/ocaml/libs/eventchn/xeneventchn_stubs.c | 29 +++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/tools/ocaml/libs/eventchn/xeneventchn_stubs.c b/tools/ocaml/libs/eventchn/xeneventchn_stubs.c
index f889a7a2e4..c0d57e2954 100644
--- a/tools/ocaml/libs/eventchn/xeneventchn_stubs.c
+++ b/tools/ocaml/libs/eventchn/xeneventchn_stubs.c
@@ -33,7 +33,30 @@
 #include <caml/fail.h>
 #include <caml/signals.h>
 
-#define _H(__h) ((xenevtchn_handle *)(__h))
+/* We want to close the event channel when it is no longer in use,
+   which can only be done safely with a finalizer.
+   Event channels are typically long lived, so we don't need tighter control over resource deallocation.
+   Use a custom block
+*/
+
+/* Access the xenevtchn_t* part of the OCaml custom block */
+#define _H(__h) (*((xenevtchn_handle**)Data_custom_val(__h)))
+
+static void stub_evtchn_finalize(value v)
+{
+	/* docs say to not use any CAMLparam* macros here */
+	xenevtchn_close(_H(v));
+}
+
+static struct custom_operations xenevtchn_ops = {
+	"xenevtchn",
+	stub_evtchn_finalize,
+	custom_compare_default, /* raises Failure, cannot compare */
+	custom_hash_default, /* ignored */
+	custom_serialize_default, /* raises Failure, can't serialize */
+	custom_deserialize_default, /* raises Failure, can't deserialize */
+	custom_compare_ext_default /* raises Failure */
+};
 
 CAMLprim value stub_eventchn_init(void)
 {
@@ -48,7 +71,9 @@ CAMLprim value stub_eventchn_init(void)
 	if (xce == NULL)
 		caml_failwith("open failed");
 
-	result = (value)xce;
+	/* contains file descriptors, trigger full GC at least every 128 allocations */
+	result = caml_alloc_custom(&xenevtchn_ops, sizeof(xce), 1, 128);
+	_H(result) = xce;
 	CAMLreturn(result);
 }
 
-- 
2.34.1



  parent reply	other threads:[~2022-07-29 17:54 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-29 17:53 [PATCH v1 0/7] tools/ocaml code and build cleanups Edwin Török
2022-07-29 17:53 ` [PATCH v1 1/7] tools/ocaml/Makefile: do not run ocamldep during make clean Edwin Török
2022-08-01  8:19   ` Christian Lindig
2022-08-03 10:16   ` Jan Beulich
2022-08-03 10:24     ` Edwin Torok
2022-08-03 10:47       ` Jan Beulich
2022-08-03 10:57       ` Anthony PERARD
2022-08-03 11:58         ` Jan Beulich
2022-08-03 12:42           ` Anthony PERARD
2022-07-29 17:53 ` [PATCH v1 2/7] tools/ocaml/*/Makefile: generate paths.ml from configure Edwin Török
2022-08-01  8:25   ` Christian Lindig
2022-08-03 10:37   ` Andrew Cooper
2022-07-29 17:53 ` [PATCH v1 3/7] tools/ocaml/*/dune: dune based build system Edwin Török
2022-08-01 10:52   ` Christian Lindig
2022-08-03 11:25   ` Anthony PERARD
2022-08-03 12:22     ` Christian Lindig
2022-07-29 17:53 ` [PATCH v1 4/7] tools/ocaml: Makefile to drive dune Edwin Török
2022-08-03 13:46   ` Anthony PERARD
2022-08-03 15:37     ` Edwin Torok
2022-08-03 17:16       ` Anthony PERARD
2022-07-29 17:53 ` [PATCH v1 5/7] tools/ocaml: fix compiler warnings Edwin Török
2022-08-01  8:23   ` Christian Lindig
2022-08-03 10:39   ` Andrew Cooper
2022-08-03 10:47     ` Christian Lindig
2022-07-29 17:53 ` [PATCH v1 6/7] tools/ocaml/libs/xb: hide type of Xb.t Edwin Török
2022-07-29 17:53 ` Edwin Török [this message]
2022-08-01  8:20   ` [PATCH v1 7/7] tools/ocaml/libs/eventchn: do not leak event channels and OCaml 5.0 compat Christian Lindig
2022-08-05 18:06   ` Andrew Cooper
2022-08-08  8:28     ` Edwin Torok
2022-08-08  9:59       ` Andrew Cooper
2022-08-01 10:49 ` [PATCH v1 0/7] tools/ocaml code and build cleanups Christian Lindig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6e5fd9edfea379b69682fa538141298fc1bc3110.1659116941.git.edvin.torok@citrix.com \
    --to=edvin.torok@citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=christian.lindig@citrix.com \
    --cc=dave@recoil.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.