You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Michael McCandless <lu...@mikemccandless.com> on 2009/11/27 17:37:57 UTC
deadlock in TestCrash
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
Re: deadlock in TestCrash
Posted by Michael McCandless <lu...@mikemccandless.com>.
True... I'll make that change too.
Mike
On Fri, Nov 27, 2009 at 12:56 PM, Jason Rutherglen
<ja...@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
> <lu...@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
Re: deadlock in TestCrash
Posted by Jason Rutherglen <ja...@gmail.com>.
If syncing on directory is not important, then sizeInBytes can be an AtomicLong?
On Fri, Nov 27, 2009 at 8:37 AM, Michael McCandless
<lu...@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