[Phil]

Thu Nov 29 05:47:55 2012 linux [...] eikelenboom.it - Ticket created
Subject:    [PATCH} PDF::API2 don't destroy pdf object on stringify or save and saves but leave it to the user
Date:    Thu, 29 Nov 2012 11:47:40 +0100
To:    bug-PDF-API2 [...] rt.cpan.org
From:    Sander Eikelenboom <linux [...] eikelenboom.it>

Hi,

At the moment the following is not working:

Code: [Select]

$pdf = PDF::API2->new();
    ...
my $filename =  md5($pdf->stringify() . "pdf");
$pdf->saveas($filename);
$pdf->end();
This is because stringify (and save and saveas) destroy the object. I think it would be better to leave this to the user and let him invoke $pdf->end();

Patch is attached.

--

Sander

#
Sat Dec 15 21:53:30 2012 steve [...] deefs.net - Correspondence added

Thank you for taking the time to write up a patch and send it in!

If we were starting from scratch, I'd agree with you.  However, if I were to implement this now, it would break quite a lot of existing code by not freeing up the memory that people are expecting to be freed (or at least accustomed to being freed).  See the comment in PDF::API2::Basic::PDF::File's "release" method to see why this is important.

However, I've made a note to clarify the documentation to the stringify method to say that it's destructive (I've been bit by this gotcha as well), and will consider other ways to implement something similar.
#
Sat Dec 15 21:53:31 2012 The RT System itself - Status changed from 'new' to 'open'
#
Sat Dec 15 21:53:31 2012 steve [...] deefs.net - Status changed from 'open' to 'rejected'

[Phil]

I would agree that the original (current) methods stringify(), save(), and saveas() should not be changed to preserve the data, as requested. However, it may be useful to add new methods (similar to the patched versions) under new names, such as stringifyCopy(), saveCopy(), and saveasCopy(). That way, the current data would be preserved for further work (apparently the intent). Other name suggestions would be welcome.

Add: (As promised above,) Steve has a Maintainer's Note in stringify() discussing the problem with circular references and the problems they cause. He suggests that perhaps the code could be restructured to avoid circular references:

# Maintainer's note: The object is being destroyed because it contains
# circular references that would otherwise result in memory not being
# freed if the object merely goes out of scope.  If possible, the
# circular references should be eliminated so that stringify doesn't
# need to be destructive.
#
# I've opted not to just require a separate call to release() because
# it would likely introduce memory leaks in many existing programs
# that use this module.
# - Steve S. (see bug RT 81530)

Does anyone know exactly what problems would arise if the data was kept rather than being destroyed? It's not clear to me just what object's going out of scope is going to result in in a memory leak.

[Phil]

Rather than new methods, we could also consider adding options like -preserveobj => true to the existing calls. The object would not be destroyed, as it is now. It would be as if, say, save() had never been called.

Before doing anything like this, we need to understand the data internals and what problems (if any) might arise from not destroying the data, as is currently done. The maintainer's note mentions that there are some circular references — would this object actually go out of scope (resulting in a memory leak), or would it act like a normal object pre-save()?

[Phil]

July 09, 2017, 2:16:34 PM by Phil

See release notes (Changes) for PDF::API2 2.032 and PDF::Builder 3.004. Steve is addressing circular references via "weaken" statements. That may help with this issue.

[Phil]

Note that this ticket is closed as rejected on the PDF::API2 queue, but is still open here. The idea is that these routines might be implemented this way under new names, so that existing code doesn't break. It's low priority, and I'll wait to see if there is any demand for such new routines before working on them.