Bug 692970 - Question about reference count
Summary: Question about reference count
Status: NOTIFIED FIXED
Alias: None
Product: GhostPCL
Classification: Unclassified
Component: PCL interpreter (show other bugs)
Version: unspecified
Hardware: PC All
: P4 normal
Assignee: Henry Stiles
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-06 14:42 UTC by Marcos H. Woehrmann
Modified: 2012-04-12 15:56 UTC (History)
1 user (show)

See Also:
Customer: 460
Word Size: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Comment 2 Henry Stiles 2012-04-09 04:44:52 UTC
The current code explicitly closes the device and does not depend on the device having the proper reference count.  We do note there is a memory leak here which needs to be fixed but the important issue is to add the call "gs_closedevice" just before where the device was freed in previous revisions of the code.  You did not provide the version number of the code in question, but have a look at the current code (pl/plmain.c:pl_main_universe_dnit()) and see how it calls gs_closedevice() and add it in a likewise manner to your previous release.   Of course, it would be best to upgrade your code.  Let me know if you have questions.
Comment 3 Horiana Costea 2012-04-10 14:24:11 UTC
The code we were running was 8.70.
However, we also ran the test on 9.05 and we noticed the same behavior:
call stack:
wmain(dwmainc.c) -> main_utf8(dwmainc.c) ->pl_main_aux(plmain.c) -> pl_main_universe_dnit(plmain.c) -> gx_device_retain(gsdevice.c)  which  doesn't call the rc_free_struct_only for reference-counted structures 
gx_device_retain contains a macro called "rc_adjust_only" which doesn't call the rc_free_struct_only because dev->rc.ref_count is 2.

we noticed for other testcases that at the same point, because dev->rc.ref_count is 1, rc_free_struct_only gets called and we will have a call stack like:

wmain  -> main_utf8  ->pl_main_aux  -> pl_main_universe_dnit  -> gx_device_retain -> rc_free_struct_only  -> chunk_free_object -> gx_device_finalize -> gs_close_device  which interests us.

Could you please explain us why in the first case reference count is still2, not 1 and is it normal not to call the rc_free_struct_only?
Comment 4 Henry Stiles 2012-04-10 14:35:18 UTC
(In reply to comment #3)
> The code we were running was 8.70.
> However, we also ran the test on 9.05 and we noticed the same behavior:
> call stack:
> wmain(dwmainc.c) -> main_utf8(dwmainc.c) ->pl_main_aux(plmain.c) ->
> pl_main_universe_dnit(plmain.c) -> gx_device_retain(gsdevice.c)  which  doesn't
> call the rc_free_struct_only for reference-counted structures 
> gx_device_retain contains a macro called "rc_adjust_only" which doesn't call
> the rc_free_struct_only because dev->rc.ref_count is 2.
> 
> we noticed for other testcases that at the same point, because
> dev->rc.ref_count is 1, rc_free_struct_only gets called and we will have a call
> stack like:
> 
> wmain  -> main_utf8  ->pl_main_aux  -> pl_main_universe_dnit  ->
> gx_device_retain -> rc_free_struct_only  -> chunk_free_object ->
> gx_device_finalize -> gs_close_device  which interests us.
> 
> Could you please explain us why in the first case reference count is still2,
> not 1 and is it normal not to call the rc_free_struct_only?

As I said in comment #2 there is a memory leak in the code which we will fix, but the code should not need to produce incorrect output because there is a memory leak, normally at that point in the program you are very close to exiting to the operating system so the memory leak does not matter.  Or if you do not exit, since the leak is fairly small, I would still think it desirable to produce the correct output and continue on.
Comment 5 Horiana Costea 2012-04-11 14:42:16 UTC
Ok, then we'll wait for the fix. 
We connected your function gs_close_device() with one of our functions that creates the end of an AFP structured field. And since gs_close_device() is not called, nor our function is called, therefore the AFP output is not complete.
Comment 6 Henry Stiles 2012-04-11 15:49:29 UTC
(In reply to comment #5)
> Ok, then we'll wait for the fix. 
> We connected your function gs_close_device() with one of our functions that
> creates the end of an AFP structured field. And since gs_close_device() is not
> called, nor our function is called, therefore the AFP output is not complete.

Right and we don't want you to do that because it is error prone.   We want you to call gs_closedevice() like it is done in the current code:


    /* close and dealloc the device if selected. */
    if (universe->curr_device) {
        gs_closedevice(universe->curr_device);
        gs_unregister_root(universe->curr_device->memory, &device_root, "pl_main_universe_select");
        gx_device_retain(universe->curr_device, false);
        universe->curr_device = NULL;


the first gs_closedevice() will call your function, the second call (a side effect of releasing the device) will not call your function because the device is closed and gs_closedevice does nothing if the device is not open.  

In sum, we want you to call gs_closedevice directly and not depend on a 0 reference count for it to be called.  Let me know if it isn't clear.
Comment 7 Henry Stiles 2012-04-12 15:41:54 UTC
Fixed with this commit

commit 1b355f679fd03a5d5d334b6d74202d9c3d58110b
Author: Henry Stiles <henry.stiles@artifex.com>
Date:   Thu Apr 12 09:34:32 2012 -0600

   Fix 692970 - device reference counting incorrect.

   The tile clipping device did not properly release its reference to the
   target device.