Re: [Xen-devel] [PATCH 11/24] golang/xenlight: define CpuidPolicyList builtin type

From: George Dunlap <george.dunlap@citrix.com>
To: Nick Rosbrook <rosbrookn@gmail.com>
Cc: Nick Rosbrook <rosbrookn@ainfosec.com>,
	Xen-devel <xen-devel@lists.xenproject.org>,
	kerriganb@ainfosec.com, Ian Jackson <ian.jackson@eu.citrix.com>,
	Wei Liu <wl@xen.org>
Subject: Re: [Xen-devel] [PATCH 11/24] golang/xenlight: define CpuidPolicyList builtin type
Date: Thu, 14 Nov 2019 17:44:16 +0000	[thread overview]
Message-ID: <9576102c-c20f-7d65-5b93-bd2632fde462@citrix.com> (raw)
In-Reply-To: <CAEBZRSeDo2RO-VptW8hYOzh93M04pxxVL16dqoAcrLGm0MbFuA@mail.gmail.com>

On 11/14/19 2:58 PM, Nick Rosbrook wrote:
>> Hmm, this introduces a pretty significant risk of memory leaks; but I
>> don't really see any way around it.  I guess we really want to do some
>> SetFinalizer() magic on this to call libxl_cpuid_dispose()?
>>
>> We might also want to add something like a .Dispose() method to have
>> predictable memory effects.  But then do we want to have a .Dispose()
>> method on all types that might contain a CpuidPolicyList?  Technically
>> we're supposed to, so we might have to. (And now I'm having deja vu,
>> like we've had this discussion before, but I can't seem to find it.)
> 
> As I've expressed before, I don't think its a good idea to look to the
> runtime to fix this sort of problem, so I'd be more inclined to look
> into a Dispose like option. But then it does seem weird from an API
> perspective to only define Dispose on some types since it introduces a
> closer, but incomplete, semantic relationship between libxl and the Go
> package.
> 
> WRT the definition of CpuidPolicyList, is the best we can do? Or is
> there a way we can hide the use of the C type better so that someone
> using this package doesn't need to worry about calling Dispose or
> otherwise? I think [1] is where we originally discussed this.

If we do have to keep the C pointer around for some reason, I think
using SetFinalizer is a necessary backstop to keep the library from
leaking.  It's all well and good to say, "Make sure you call Dispose()",
but I think for a GC'd language that's just going to be too easy to
forget; and it will be a huge pain for long-running processes.

It is pretty annoying; and this is really the *only* type that has this
"opaque structure behind a pointer" property.

If we didn't have this type as a type, we'd have to avoid somehow
exposing the user to the functions which take and use it.  The main
place it's used ATM is in DomainBuildInfo.  We could explore whether it
would be practical to "implement" CpuidPolicyList as a string, and then
have toC() call libxl_cpuid_parse_config().  Obviously that means
fromC() would fail; but I'm not sure DomainBuildInfo is really a
structure passed "out" of libxl anyway.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel