[Lucene Bug] Did not decrement max count

[Fuxiang Chen <fc...@cse.ust.hk>]

Dear Lucene Developers,

In the source file "SolrXMLSerializer.java", there is a flaw in the code
fragment that is using transferTo.

Original Code
--------------------
      long size = fcin.size();
      long position = 0;
      while (position < size) {
        position += fcin.transferTo(position, MB32, fcout);
      }

The MB32 (which is the max count needs to be decrement.

See reference: http://stackoverflow.com/questions/7379469

Changed Code
----------------------
while (position < size) {
    long count = inChannel.transferTo(position, MB32, fcout);
    if (count > 0)
    {
        position += count;
        MB32 -= count;
    }
}


Please consider fixing this bug.

Thanks!


-- 
Warmest Regards,
    Fuxiang

Re: [Lucene Bug] Did not decrement max count

[Chris Hostetter <ho...@fucit.org>]

: In the source file "SolrXMLSerializer.java", there is a flaw in the code
: fragment that is using transferTo.

Nothing in your email, or in the link you cited, explains why you think 
this is a bug...

: The MB32 (which is the max count needs to be decrement.
: 
: See reference: http://stackoverflow.com/questions/7379469

The discussion on that URL is specifically about people attempting to copy 
entire files via a *single* call to transferTo/transferFrom.  2 main 
things are discussed:

1) the importance of making transfertTo/transferFrom calls in a loop  
that inspects the return value to see how much data was actaully copied so 
you know the position to use for hte next transfer.

2) the possibility of bugs on some platforms if you attempt to copy more 
then 64MB at a time 

...neither of which are a problem in SolrXMLSerializer.java (the code 
already checks the return value nad loops & the max size to copy is fixed 
at 32 MB)

One user on the URL you cited suggested decrementing the the number of 
bytes copied in a loop each time, w/o citing any justification for why 
that should be done -- a subsequent user followed up with a comment 
suggesting that it only makes sense if the initial number of bytes is very 
large ("AFAIK, maxCount should be decremented only if it is given the 
value of the file size in the very beginning: maxCount = filesize, and 
then decrement it in each loop") but again: no explanation or 
justification for why this might be is provided.


The code you suggest we use looks very dangerous to me -- anytime 
a file larger then 32 MB gets copied if transferTo does in fact copy a 
full 32 MB on the first method call, then the decrement will result in the 
next call asking for 0 bytes to be copied. 

Your email does have me wondering however: why does this method exist in 
SolrXMLSerializer? ... it seems like common-io FileUtils.copyFile should 
be used instead, so i've opened a jira for that...

https://issues.apache.org/jira/browse/SOLR-6339



: 
: Changed Code
: ----------------------
: while (position < size) {
:     long count = inChannel.transferTo(position, MB32, fcout);
:     if (count > 0)
:     {
:         position += count;
:         MB32 -= count;
:     }
: }
: 
: 
: Please consider fixing this bug.
: 
: Thanks!
: 
: 
: -- 
: Warmest Regards,
:     Fuxiang
: 

-Hoss
http://www.lucidworks.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org