[BF] slab init + http file cache fixes

[Sergey Brester]

> Hello!
> 
> On Wed, Jun 15, 2016 at 01:53:31PM +0200, Sergey Brester wrote:
> 
> Thanks, looks like another win32-related problem.

Hmm... Grrr...

> This should be two separate patches.

Ok

> Not sure it's a good idea to reuse min_uses return code here. I
> would rather not.

Otherwise the cacheable requests will fail, just while the zone don't 
get a free node (as long as scarce).
Because in other respects I do not understand why in other scarce cases 
"ngx_http_file_cache_open" will return NGX_HTTP_CACHE_SCARCE and 
continue processing of the request.

...
> The condition added looks wrong: fcn->deleting cannot be set
> here, as it's only set by this function under lock, and
> fcn->count++ here will prevent the function from being called
> again.
> 
> As far as I understand, you are trying to change processing of the
> race which happens if the node in question is grabbed by another
> request while the file is being deleted (fcn->count != 0 at
> the function end). Please explain what you are trying to chanage
> and how it's expected to affect various cases possible.

Do you really understand what the race condition means? And you see a 
pair lines bellow an ngx_shmtx_unlock for pool, so the second worker can 
wins and goes this block again?

I can only add, that without this fix I had observed:
1) negative value of "cache->sh->size", what evidence, that the count of 
increment is fewer as decrement (same response length, so always the 
same "fcn->fs_size" and no cache update);
2) sporadically errors by deleting of cache files;


> +        goto done;
> ...
> This change looks unrelated. If you think it's needed, please
> explain the reasons.

But sure. Otherwise in many cases (e.g. case of scarce) the value of 
"cache->files" will be incremented (with ++cache->files).


> See above, the description doesn't match style and doesn't
> describe that this is a win32 problem.

I would be not so sure by win32. At least the code is very strange and 
error-prone (for example if tomorrow some child processes will be not 
forked and not executed stub initialization...


> ngx_slab_core_init ...
> I can't say I like the name, but I have no better suggestions.

What thus you want to tell me?


> ...
> This test is useless with the new logic you've introduced.
> ...
> This call is useless with the new logic you've introduced.

Both are done if someone uses slab module as API (as module pure outside 
nginx process), so not required to call ngx_slab_core_init extra.
But I can remove it if disturbing (I wanted to leave it just because 
this one will be called once per process).


----

BTW. I've always the same questions if I look how some people take 
contributions:

   Why such people often assume, that:
   - in contrast to them, the contributor have all the time in the world
     (e.g. to revise and resend an update again and again)?
   - do us a favor, if they assume the commit?
   - think to be the power holder of the project, that is an open source 
project,
     that in the meantime the community of the whole world has 
(co-)developed?
   - they are always smarter than those who have found the bug and fixed 
it?

Merely as the signal to cogitate...