Re: svn commit: r1797422 - /apr/apr/trunk/memcache/apr_memcache.c

[Yann Ylavic]

On Mon, Jun 5, 2017 at 10:32 PM, William A Rowe Jr  wrote:
> What happens if two different threads attempt this poll in parallel? I
> presumed pollsets are copied to prevent two threads from clashing.

APR_POLLSET_NOCOPY is about the lifetime of the pollfds, while
thread-safety should be ensured by APR_POLLSET_THREADSAFE (using
no/zero flags as before was actually not thread-safe already).

BTW, if the OS/native pollset is already threadsafe and
APR_POLLSET_NOCOPY is specified, APR_POLLSET_THREADSAFE can sometimes
be omitted/ignored (this the case for e.g. epoll, but not for kqueue
whereas it possibly could be).
Hence if we really need to be thread-safe here we should use
APR_POLLSET_THREADSAFE explicitely.

Re: svn commit: r1797422 - /apr/apr/trunk/memcache/apr_memcache.c

[Eric Covener]

On Mon, Jun 5, 2017 at 4:32 PM, William A Rowe Jr  wrote:
> What happens if two different threads attempt this poll in parallel? I
> presumed pollsets are copied to prevent two threads from clashing.

>>  pollfds = apr_pcalloc(temp_pool, apr_hash_count(server_queries) *
>> sizeof(apr_pollfd_t));
>>
>> -rv = apr_pollset_create(, apr_hash_count(server_queries),
>> temp_pool, 0);
>> +rv = apr_pollset_create(, apr_hash_count(server_queries),
>> temp_pool,
>> +APR_POLLSET_NOCOPY);

Looks kosher, the pollfds that will be added to this pollset have the
same lifetime / same pool (beginning of the diff context), and the
pollset does not escape this function.

Re: svn commit: r1797422 - /apr/apr/trunk/memcache/apr_memcache.c

[William A Rowe Jr]

What happens if two different threads attempt this poll in parallel? I
presumed pollsets are copied to prevent two threads from clashing.


On Jun 2, 2017 14:58,  wrote:

> Author: jailletc36
> Date: Fri Jun  2 19:58:55 2017
> New Revision: 1797422
>
> URL: http://svn.apache.org/viewvc?rev=1797422=rev
> Log:
> Save some cycles by not copying the pollfds.
>
> Modified:
> apr/apr/trunk/memcache/apr_memcache.c
>
> Modified: apr/apr/trunk/memcache/apr_memcache.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/memcache/apr_
> memcache.c?rev=1797422=1797421=1797422=diff
> 
> ==
> --- apr/apr/trunk/memcache/apr_memcache.c (original)
> +++ apr/apr/trunk/memcache/apr_memcache.c Fri Jun  2 19:58:55 2017
> @@ -1296,7 +1296,8 @@ apr_memcache_multgetp(apr_memcache_t *mc
>  /* create polling structures */
>  pollfds = apr_pcalloc(temp_pool, apr_hash_count(server_queries) *
> sizeof(apr_pollfd_t));
>
> -rv = apr_pollset_create(, apr_hash_count(server_queries),
> temp_pool, 0);
> +rv = apr_pollset_create(, apr_hash_count(server_queries),
> temp_pool,
> +APR_POLLSET_NOCOPY);
>
>  if (rv != APR_SUCCESS) {
>  query_hash_index = apr_hash_first(temp_pool, server_queries);
>
>
>