Hi again,
Well, it seems like this won't work because at the end of
"storeCossCreate" it has:
" storeCossMemBufLock(SD, sio);
coss_stats.create.success++;
return sio;
}
On store_io_coss.c:storeCossClose, there's a comment:
/*
* Aha! The unlocked membuf.
*
* If its storeCossCreate, then it was locked. Fine.
* If it was storeCossOpen() and we found the object in-stripe then cool,
* its locked.
* If it was storeCossOpen() and we didn't find the object in-stripe then
* we reallocated the object into the current stripe and locked THAT.
*/
This "if its storeCossCreate" statement is no longer true if it
return, so I think the patch will make squid do a segmentation fault
on storeCossClose...although if storeCossCreate returns NULL if object
was _not_ created ergo shouldn't be closed, right?
Since it takes a few days to see if the error occurs, I'm trying to
see why the patch is not a good solution.
I'm attaching the patch inline since gmail appears to do a MIME conversion..
Any pointers if I'm going on the right direction would be great :-)
Cheers,
- Robert
Patch agains squid7-stable9:
diff -urN a/src/fs/coss/store_io_coss.c b/src/fs/coss/store_io_coss.c
--- a/src/fs/coss/store_io_coss.c 2006-11-05 19:14:32.000000000 -0200
+++ b/src/fs/coss/store_io_coss.c 2011-01-10 15:25:22.452889227 -0200
@@ -300,6 +300,9 @@
CossInfo *cs = SD->fsdata;
assert(cs->rebuild.rebuilding == 0);
+ sfileno swap_filen = storeCossAllocate(SD, e, COSS_ALLOC_ALLOCATE);
+ if (swap_filen == -1)
+ return NULL;
coss_stats.create.ops++;
sio = cbdataAlloc(storeIOState);
cstate = memPoolAlloc(coss_state_pool);
@@ -319,9 +322,8 @@
*/
sio->st_size = objectLen(e) + e->mem_obj->swap_hdr_sz;
sio->swap_dirn = SD->index;
- sio->swap_filen = storeCossAllocate(SD, e, COSS_ALLOC_ALLOCATE);
+ sio->swap_filen = swap_filen;
debug(79, 3) ("storeCossCreate: %p: filen: %d\n", sio, sio->swap_filen);
- assert(-1 != sio->swap_filen);
sio->callback = callback;
sio->file_callback = file_callback;
2011/1/10 Robert Pipca <robertpipca@xxxxxxxxx>:
> Hi Mr. Jeffreis,
>
> I'm no squid programmer, but I'm trying to see if the attached patch will work.
>
> The aufs implementation of (obj.create) returns NULL of the operation
> fails, so I'm changing the assert to an if and hopes that this fixes.
>
> But the COSS code uses the (obj.create) functions on other parts of
> the code, and is not clear to me that it checks for NULL (though I
> think it doesn't ).
>
> Cheers,
>
> - Robert
>
> 2011/1/7 Robert Pipca <robertpipca@xxxxxxxxx>:
>> Hi again,
>>
>> 2011/1/7 Robert Pipca <robertpipca@xxxxxxxxx>:
>>> This assertion failed occurs on all of them. So, Mr. Henrik, can you
>>> help hunt down this bug on squid like mr. Jeffreis suggested?
>>
>> On trying to track down the bug, I put a lot of "debug (1,1) ("shit
>> happened = %d\n", __LINE__);" of all possible code-paths of returning
>> -1 in storeCossAllocate (fs/coss/store_io_coss.c).
>>
>> For all I can see, all of the -1 returns of storeCossAllocate are
>> because storeCossCreateMemBuf returns NULL.
>>
>> Does this help?
>>
>> Cheers,
>>
>> - Robert
>>
>
