| |
|
 |
|
 |
|
|
|
| |
|
|
xen-devel
Re: [Xen-devel] Re: [PATCH 0 of 3] xl: free allocations made at top leve
On Fri, 2010-07-30 at 15:21 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] Re: [PATCH 0 of 3] xl: free allocations
> made at top level"):
> > This seems to work. I'm not entirely sure about it though -- it's not
> > clear what happens if the thread which calls xc_interface_close is not
> > the final thread to exit and whether we would still leak the buffer from
> > the thread which does the final exit() call.
>
> Yers. We don't impose a requirement that the thread that calls
> xc_interface_open has to be the one which calls _close, so you can
> still leak the buffer from the original thread.
Unless that exists with pthread_exit, I suppose.
TBH I think we needn't worry too much about this particular leak -- for
threads which do come and go they data will be freed, and for the last
thread which exists it's not really a worry.
The patch is enough to make a single threaded user like xl leak free
which is useful for the purposes of spotting any more serious leaks.
> OTOH if you don't want memory leaks you do need to do
> xc_interface_close. So perhaps we should have a reference count or
> something ?
As long as the handle is either opened once and closed on exit or each
thread opens and closes its own handle I think everything is ok.
> Is it even possible to destroy thread-specific data for
> another thread ?
I don't think so, but I don't think its a concern in this case since all
threads will either clean up after themselves when they are done or the
whole process is exiting anyway.
It's also fine to free the thread-specific data too eagerly, worst case
the next attempts at a hypercall will have to reallocate it.
> Also, while I was reading this code I noticed this:
>
> pthread_once(&hcall_buf_pkey_once, _xc_init_hcall_buf);
>
> and:
>
> static void _xc_init_hcall_buf(void)
> {
> pthread_key_create(&hcall_buf_pkey, _xc_clean_hcall_buf);
> }
>
> It seems to me that this is possibly racy, if two threads call
> hcall_buf_prep simultaneously for the first time (which is not at all
> implausible in some possible scenarios). pthread_once doesn't seem to
> guarantee that the first two calls arrive at once, the 2nd call
> doesn't return before the first call has even entered the user
> function. So the thread which loses the race can read an
> uninitialised hcall_buf_pkey.
Looking at the glibc source code it looks like the loser of the race
will wait, judging from the comment "/* Somebody else got here first.
Wait. */" and the subsequent sys_futex call in the assembly (without
studying it too hard).
I'm not sure pthread_once would be at all useful without these
semantics. I guess that doesn't mean those aren't the specified
semantics though ;-)
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
|
|
| |
|
Home