samba 3.6.0rc2 question

[Herb Lewis]

bug 8307 filed

Jeremy Allison wrote:
> On Thu, Jul 14, 2011 at 05:42:26PM -0700, Jeremy Allison wrote:
>> On Wed, Jul 13, 2011 at 03:34:58PM -0700, Herb Lewis wrote:
>>> I'm trying to use the vfs functions for brl_lock_windows and
>>> brl_unlock_windows however it doesn't seem like the brl_unlock_windows
>>> function is getting called on file close unless posiz locks is set.
>>>
>>> locking_close_file is called which in turn calls brl_close_fnum but
>>> this only calls brl_unlock which is what calls SMB_VFS_BRL_UNLOCK_WINDOWS
>>> within an if(lp_posix_locking(fsp->conn->params)) clause.
>>>
>>> This seems to be a bug as the vfs unlock routine will never get called
>>> to release the locks on close. I'm not quite sure where this fix needs
>>> to go though.
>> Oh - I see the problem ! We're getting bitten by an optimization
>> bug which would only bite if you've hooked into the SMB_VFS_BRL_UNLOCK_WINDOWS
>> call.
>>
>> The code inside brl_close_fnum() is taking a shortcut if we are not
>> mapping onto POSIX locks inside smbd.
>>
>> In that case it "knows" that locks are only stored in the tdb mapped
>> into memory and so bulk deletes them from the copy of this array
>> stored on the br_lck struct and re-stores them - WITHOUT calling
>> the VFS call on each unlock.
>>
>> It might be that we can just remove this optimization completely.
>> Give me a little while to create a patch that does this that you
>> can use. I'll investagate what really needs to get fixed here.
> 
> Here is a version (for v3-6-test) that just completely removes all the optimization
> and calls brl_unlock() for every lock on file close. Passes all tests
> and seems to be the thing to push for master.
> 
> If you can log a bug on this I'll get it scheduled for 3.6.1 (it's
> too late for 3.6.0 I'm afraid).
> 
> It really does seem that premature optimization is the root of all
> evil :-).
> 
> Cheers,
> 
> Jeremy.
>