[Gluster-devel] readdir() harmful in threaded code

[Niels de Vos]

On Mon, Jul 25, 2016 at 05:43:30AM +0200, Emmanuel Dreyfus wrote:
> Vijay Bellur <vbellur at redhat.com> wrote:
> 
> > Do you have any concrete examples of problems encountered due to the 
> > same directory stream being invoked from multiple threads?
> 
> I am not sure this scenario can happen, but what we had were directory
> offsets reused among different DIR * opened on the same directory. This
> works on Linux but is a standard violation, as directory offsets are
> supposed to be valid only for a given DIR *.
> 
> It broke NetBSD regression enough that I added a test against it in
> xlator/storage/posix/src:posix.c
> 
>                 seekdir (dir, off); 
> #ifndef GF_LINUX_HOST_OS        
>                 if ((u_long)telldir(dir) != off && off != pfd->dir_eof)
> {
>                         gf_msg (THIS->name, GF_LOG_ERROR, EINVAL,
>                                 P_MSG_DIR_OPERATION_FAILED,
>                                 "seekdir(0x%llx) failed on dir=%p: "
>                                 "Invalid argument (offset reused from "
>                                 "another DIR * structure?)", off, dir);
>                         errno = EINVAL; 
>                         count = -1;
>                         goto out;
>                 }
> #endif /* GF_LINUX_HOST_OS */
> 
> 
> 
> About standards and portability, here is the relevant part in Linux man
> page: 
> > In the current POSIX.1 specification (POSIX.1-2008), readdir(3) is not
> > required to be thread-safe.  However, in modern implementations
> > (including the glibc implementation), concurrent calls to readdir(3)
> > that specify different directory streams are thread-safe.
> > 
> > It is expected that a future version of POSIX.1 will make readdir_r()
> > obsolete, and require that readdir() be thread-safe when concurrently
> > employed on different directory streams.
> 
> This means linux recommands using readir(), but such practice is likely
> to break on other systems, since standards do not currently requires it
> to be thread-safe. We can go the readdir() way, but pleas add locks.
> 
> Alternatively we can use #ifdef to use alternate code on Linux (readdir)
> and others (readdir_r)

This 2nd approach is indeed what Kaleb implemented in
http://review.gluster.org/14838 and has mentioned in an other reply in
the thread.

HTH,
Niels
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20160725/71c72173/attachment.sig>