[ruby-core:83782] Re: [Ruby trunk Bug#14108] Seg Fault with MinGW on svn 60769

From: Eric Wong <normalperson@...>
Date: 2017-11-15 09:20:04 UTC
List: ruby-core #83782
"U.NAKAMURA" <usa@garbagecollect.jp> wrote:
> Hi, Eric,
> 
> In message "[ruby-core:83779] Re: [Ruby trunk Bug#14108] Seg Fault with MinGW on svn 60769"
>   on Wed, 15 Nov 2017 07:12:42 +0000, normalperson@yhbt.net wrote:
> > > You can see our implementation of readdir on Windows in
> > > win32/win32.c (rb_w32_readdir).
> > > I doubt the second argument (rb_encoding *enc) is the cause.
> 
> As Nobu said at [ruby-core:83776] (thank you Nobu!), my guess was wrong
> and the cause was touching GVL before running VM.
> Therefore r60770 is the right way to resolve the problem.

Right, that was a real bug, too.

> > Right, enc is no problem.  The problem is using DIR * from stdio
> > is not reentrant or thread-safe.  readdir_r is deprecated, even,
> > and so it is up to the application to provide thread-safety when
> > reading directories.
> 
> Our readdir implementation for Win32 is expected to be thread-safe.
> And, it seems that there is no platform which uses non thread-safe
> readdir.

OK, but do we know all platforms?  Reading glibc, it seems
readdir(3) is thread-safe (but not reentrant) at dirstream level
(see below)

> > Usually on modern platforms (but not guaranteed), readdir on
> > different DIR * pointers is safe from multiple threads, so maybe
> > we can add a mutex to "struct dir_data".
> > 
> > I will revert r60772, r60770, and r60769 for now...
> 
> So, IMO you don't need to revert them.
> (but this mail is too late...)

Linux readdir(3) man-page says:

  In  the  current POSIX.1 specification (POSIX.1-2008), readdir() is not
  required  to  be  thread-safe.   However,  in  modern   implementations
  (including  the  glibc  implementation),  concurrent calls to readdir()
  that specify different directory streams  are  thread-safe.   In  cases
  where  multiple threads must read from the same directory stream, using
  readdir() with external synchronization is still preferable to the  use
  of  the deprecated readdir_r(3) function.  It is expected that a future
  version of POSIX.1 will require that readdir() be thread-safe when con‐
  currently employed on different directory streams.

So based on that paragraph alone, my revert seems correct.
Ruby Dir#read method can be called in multiple threads on the
same Dir object.  That is dangerous.

However; when reading glibc source[1], I noticed every DIR stream has
a lock which guards readdir calls since 1996[2].

So maybe whitelist glibc (and maybe win32) implementations
for GVL release on readdir?


[1] git://sourceware.org/git/glibc.git - sysdeps/posix/readdir.c
[2] commit c150923988933b5db75a974d4cc08cd7f7aaf3dc in glibc[1]

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>

In This Thread