lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael McCandless <luc...@mikemccandless.com>
Subject Re: deadlock in TestCrash
Date Fri, 27 Nov 2009 18:29:03 GMT
True... I'll make that change too.

Mike

On Fri, Nov 27, 2009 at 12:56 PM, Jason Rutherglen
<jason.rutherglen@gmail.com> wrote:
> If syncing on directory is not important, then sizeInBytes can be an AtomicLong?
>
> On Fri, Nov 27, 2009 at 8:37 AM, Michael McCandless
> <lucene@mikemccandless.com> wrote:
>> RAMFile has this method:
>>
>>  final synchronized byte[] addBuffer(int size) {
>>    byte[] buffer = newBuffer(size);
>>    if (directory!=null)
>>      synchronized (directory) {             // Ensure addition of
>> buffer and adjustment to directory size are atomic wrt directory
>>        buffers.add(buffer);
>>        directory.sizeInBytes += size;
>>        sizeInBytes += size;
>>      }
>>    else
>>      buffers.add(buffer);
>>    return buffer;
>>  }
>>
>> But in working on LUCENE-2095, I'm seeing a deadlock, because that
>> method first syncs on RAMFile and then tries to sync on directory,
>> while the MockRAMDirectory.crash method, and I think maybe other
>> places, do the reverse.  I remember also hitting a different deadlock
>> from this in the past, and working around it.
>>
>> I'd like to break the deadlock by changing RAMFile to this:
>>
>>  final byte[] addBuffer(int size) {
>>    byte[] buffer = newBuffer(size);
>>    synchronized(this) {
>>      buffers.add(buffer);
>>      sizeInBytes += size;
>>    }
>>
>>    if (directory != null) {
>>      synchronized (directory) {
>>        directory.sizeInBytes += size;
>>      }
>>    }
>>    return buffer;
>>  }
>>
>> But, then, the addition of the buffer and the change of sizeInBytes is
>> no longer atomic wrt the directory (as the comment says).
>>
>> So my question is... is this change OK?  Why is/was it so crucial that
>> the sizeInBytes & RAMFile's buffers really change only atomically?
>> Ie, it seems harmless if RAMDirectory.sizeInBytes() might not include
>> a buffer that's in the process of being added...?
>>
>> (I do see TestRAMDirectory.testRAMDirectorySize requires this
>> atomicity, but, I can fix the test to only check the size in the
>> end...).
>>
>> Mike
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
>> For additional commands, e-mail: java-dev-help@lucene.apache.org
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-dev-help@lucene.apache.org
>
>

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


Mime
View raw message