Mailing List Archive: Perl: porters

[PATCH] Re: [PATCH] abstract mempool header testing

Index | Next | Previous | View Threaded

jhi at iki

Nov 27, 2006, 9:25 AM

Post #1 of 7 (292 views)
Permalink

[PATCH] Re: [PATCH] abstract mempool header testing

Jarkko Hietaniemi wrote:
> I still don't know whether the mempool header testing for
> being in the same mempool (in perlio.c) is the right thing
> to do but to combat cut-and-pastitis (same code in op.c)
> here's a patch that abstracts this testing a bit.

The attached patch replaces the earlier one and it adds not doing
the PERL_TRACK_MEMPOOL if the PerlMemShared* are just plain malloc
et alia. What this means is that the PERL_TRACK_MEMPOOL is *NOT*
done in non-Win32/NetWare. I didn't tie the logic to
PERL_IMPLICIT_SYS as such, though I was much tempted to
(see
http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2006-11/msg00994.html
for additional pointers)

It is very possible that this patch is not quite right, but at least
it removes the valgrind memcheck hit, but I really still do not quite
get the PERL_TRACK_MEMPOOL stuff, and I really do hope Jan Dubois will
chime in at some point to resolve this mess.

Attachments:

mempool2.pat (1.24 KB)

jhi at iki

Nov 27, 2006, 2:36 PM

Post #2 of 7 (272 views)
Permalink

Re: [PATCH] Re: [PATCH] abstract mempool header testing [In reply to]

Jarkko Hietaniemi wrote:
> Jarkko Hietaniemi wrote:
>> I still don't know whether the mempool header testing for
>> being in the same mempool (in perlio.c) is the right thing
>> to do but to combat cut-and-pastitis (same code in op.c)
>> here's a patch that abstracts this testing a bit.
>
> The attached patch replaces the earlier one and it adds not doing
> the PERL_TRACK_MEMPOOL if the PerlMemShared* are just plain malloc
> et alia. What this means is that the PERL_TRACK_MEMPOOL is *NOT*
> done in non-Win32/NetWare. I didn't tie the logic to
> PERL_IMPLICIT_SYS as such, though I was much tempted to
> (see
> http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2006-11/msg00994.html
> for additional pointers)
>
> It is very possible that this patch is not quite right, but at least
> it removes the valgrind memcheck hit, but I really still do not quite
> get the PERL_TRACK_MEMPOOL stuff, and I really do hope Jan Dubois will
> chime in at some point to resolve this mess.

Rats and barnacles, faulty patch. Try this one.

Attachments:

mempool2.pat (3.80 KB)

davem at iabyn

Nov 28, 2006, 3:42 AM

Post #3 of 7 (274 views)
Permalink

Re: [PATCH] Re: [PATCH] abstract mempool header testing [In reply to]

On Mon, Nov 27, 2006 at 12:25:53PM -0500, Jarkko Hietaniemi wrote:
> The attached patch replaces the earlier one and it adds not doing
> the PERL_TRACK_MEMPOOL if the PerlMemShared* are just plain malloc
> et alia.

But I thought the whole point of PERL_TRACK_MEMPOOL was to simulate the
behaviour of fussy systems like win32, so that people who do their
development on UNIX don't write code that breaks on windows?

--
The Enterprise's efficient long-range scanners detect a temporal vortex
distortion in good time, allowing it to be safely avoided via a minor
course correction.
-- Things That Never Happen in "Star Trek" #21

jhi at iki

Nov 29, 2006, 6:01 AM

Post #4 of 7 (267 views)
Permalink

Re: [PATCH] Re: [PATCH] abstract mempool header testing [In reply to]

Dave Mitchell wrote:
> On Mon, Nov 27, 2006 at 12:25:53PM -0500, Jarkko Hietaniemi wrote:
>> The attached patch replaces the earlier one and it adds not doing
>> the PERL_TRACK_MEMPOOL if the PerlMemShared* are just plain malloc
>> et alia.
>
> But I thought the whole point of PERL_TRACK_MEMPOOL was to simulate the
> behaviour of fussy systems like win32, so that people who do their
> development on UNIX don't write code that breaks on windows?

So thought I. But I cannot see how that can work at the moment.
But I am hopefully wrong, and hopefully Jan will pipe up soon.
(My intelligence sources say that might happen soon.)

nick at ccl4

Nov 29, 2006, 7:16 AM

Post #5 of 7 (266 views)
Permalink

Re: [PATCH] Re: [PATCH] abstract mempool header testing [In reply to]

On Wed, Nov 29, 2006 at 09:01:18AM -0500, Jarkko Hietaniemi wrote:
> Dave Mitchell wrote:
> > On Mon, Nov 27, 2006 at 12:25:53PM -0500, Jarkko Hietaniemi wrote:
> >> The attached patch replaces the earlier one and it adds not doing
> >> the PERL_TRACK_MEMPOOL if the PerlMemShared* are just plain malloc
> >> et alia.
> >
> > But I thought the whole point of PERL_TRACK_MEMPOOL was to simulate the
> > behaviour of fussy systems like win32, so that people who do their
> > development on UNIX don't write code that breaks on windows?
>
> So thought I. But I cannot see how that can work at the moment.
> But I am hopefully wrong, and hopefully Jan will pipe up soon.
> (My intelligence sources say that might happen soon.)

Well, I'm not Jan, but as I understand it, if you have PERL_TRACK_MEMPOOL
defined then this structure:

struct perl_memory_debug_header {
tTHX interpreter;
# ifdef PERL_POISON
MEM_SIZE size;
# endif
struct perl_memory_debug_header *prev;
struct perl_memory_debug_header *next;
};

is stored before every memory allocation with New/Renew etc (by increasing
the allocation request from the real system malloc, writing that structure
at the head, then returning a pointer to the byte after the structure).
The prev and next pointers are used for a doubly-linked-list to hang all
the allocations from that ithread together.

At Safefree() time a check is made that you're in the same interpreter.
(Else you panic)

At ithread closedown the interpreter chases round the linked list and (system)
free()s any remaining memory. So if you're using something like valgrind it
will spot if you're still reading it too late.

With PERL_POISON also defined memory is scribbled on before it is freed.

I think that's all.

Nicholas Clark

jhi at iki

Nov 29, 2006, 7:46 AM

Post #6 of 7 (269 views)
Permalink

Re: [PATCH] Re: [PATCH] abstract mempool header testing [In reply to]

> Well, I'm not Jan, but as I understand it, if you have PERL_TRACK_MEMPOOL
> defined then this structure:

Well, I'm not Jan either :-) and what you describe I am aware of
(but thanks for the clear explanation). What I'm saying is this:
if the below is compiled in a non-Win32 box (in this particular
case, an Ubuntu x86) with Configure -Duseithreads -Doptimize=-g:

void
foo()
{
void*p=PerlMemShared_malloc(42);
PerlMemShared_free(p);
}

the result of macro expansion leaves us with:

void
foo()
{
void*p=malloc(42);
free((p));
}

In other words, the PerlMemShared* fall back to vanilla malloc family
unless the PERL_IMPLICIT_SYS is defined (which it is only in Win32
and the like, methinks).

Now in S_more_refcounted_fds() we do PerlMemShared_realloc(),
and in PerlIO_teardown() we do PerlMemShared_free().

But since the realloc was really a plain realloc(), there is no
struct perl_memory_debug_header in front of the real memory block,
regardless of whether there was PERL_TRACK_MEMPOOL in effect or not.
So we cannot peek "in front" of the real memory block.

> struct perl_memory_debug_header {
> tTHX interpreter;
> # ifdef PERL_POISON
> MEM_SIZE size;
> # endif
> struct perl_memory_debug_header *prev;
> struct perl_memory_debug_header *next;
> };
>
> is stored before every memory allocation with New/Renew etc (by increasing
> the allocation request from the real system malloc, writing that structure
> at the head, then returning a pointer to the byte after the structure).
> The prev and next pointers are used for a doubly-linked-list to hang all
> the allocations from that ithread together.
>
> At Safefree() time a check is made that you're in the same interpreter.
> (Else you panic)
>
> At ithread closedown the interpreter chases round the linked list and (system)
> free()s any remaining memory. So if you're using something like valgrind it
> will spot if you're still reading it too late.
>
> With PERL_POISON also defined memory is scribbled on before it is freed.
>
> I think that's all.
>
> Nicholas Clark
>

jand at activestate

Nov 29, 2006, 11:55 AM

Post #7 of 7 (265 views)
Permalink

RE: [PATCH] Re: [PATCH] abstract mempool header testing [In reply to]

> > Well, I'm not Jan, but as I understand it, if you have PERL_TRACK_MEMPOOL
> > defined then this structure:
>
> Well, I'm not Jan either :-) and what you describe I am aware of

Ok, I have to admit, I'm Jan...

Sorry for being so slow in following up on this!

The way I see this, PERL_TRACK_MEMPOOL only applies to memory allocated by
Perl_safe...alloc(). These allocation may come from a per-interpreter heap.

These per-interpreter heaps allow embedding applications (like mod_perl) to
periodically recycle Perl interpreters and not worry too much about leaking
SVs etc because the whole memory pool would be freed, regardless of any
remaining suballocations within.

The memory allocated by PerlMemShared_...() on the other hand is always
global, it is not being reclaimed automatically when an interpreter is
destroyed. So there should be *no* PERL_TRACK_MEMPOOL code in places that
modify these global memory areas.

[...]

> Now in S_more_refcounted_fds() we do PerlMemShared_realloc(),
> and in PerlIO_teardown() we do PerlMemShared_free().
>
> But since the realloc was really a plain realloc(), there is no
> struct perl_memory_debug_header in front of the real memory block,
> regardless of whether there was PERL_TRACK_MEMPOOL in effect or not.
> So we cannot peek "in front" of the real memory block.

There would never be a perl_memory_debug_header in front of this block
unless someone went in and added similar code to the PerlMemShared_...()
APIs. I do not think this is necessary.

I think all that is needed is removal of the PERL_TRACK_MEMPOOL code
from PerlIO.c The PERL_TRACK_MEMPOOL stuff should be local to util.c;
essentially an implementation detail of the Perl_safe...() API.

Cheers,
-Jan

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact lists@gossamer-threads.com