[ 
https://issues.apache.org/jira/browse/LUCY-274?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14314541#comment-14314541
 ] 

Marvin Humphrey commented on LUCY-274:
--------------------------------------

The danger of starting out with a refcount of 2 is that it puts us close to 4,
which will trigger lazy cached host object initialization for RawPosting
objects.

I was worried that this would introduce the possibility of memory errors: if
the MemoryPool goes away and releases the memory for all the RawPostings it
has allocated, what happens when the Perl objects associated with those
RawPostings get DESTROYed during global destruction, no longer referring to
valid Clownfish objects?

However, it turns out that with the present implementation, the generated XS
DESTROY wrapper doesn't dereference the object, and if RawPost_Destroy_IMP
doesn't touch the object either we are safe.  (Relying on CFC's XS code
generation implementation introduces a potential action-at-a-distance bug and
isn't best practice, but none of the immediate solutions are ideal.)

So, the "only" potential downside in the short term is that if some Lucy
analysis chain happens to increase the refcounts of RawPosting objects too
high, there will be a massive memory leak of SVs followed by a tsunami of
error messages at global destruction.

I hacked in a check to see how high refcounts go for RawPosting objects when
the Lucy test suite runs and it turns out that they never get incref'd even
once.  So I think we'd probably be OK for core analyzers at least.

In conclusion, I'm OK with your suggestion because it's probably fine and the
failure mode is obvious and not a memory error.  I'll make a new pull request.
Let me know if you spot any flaws in my reasoning.


> Refcounting needs to use functions rather than methods
> ------------------------------------------------------
>
>                 Key: LUCY-274
>                 URL: https://issues.apache.org/jira/browse/LUCY-274
>             Project: Lucy
>          Issue Type: Improvement
>          Components: Index, Test, Util
>            Reporter: Marvin Humphrey
>            Assignee: Marvin Humphrey
>
> As of CLOWNFISH-27, refcounting is no longer overrideable.  Lucy needs to use 
> the new API and to eliminate the overriding of refcount manipulation methods 
> for RawPosting.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to