Re: [Boost-bugs] [Boost C++ Libraries] #12578: Crash with boost::filesystem's directory_iterator and recursive_directory_iterator

Subject: Re: [Boost-bugs] [Boost C++ Libraries] #12578: Crash with boost::filesystem's directory_iterator and recursive_directory_iterator
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2016-11-22 14:46:16

#12578: Crash with boost::filesystem's directory_iterator and
recursive_directory_iterator
-------------------------------+------------------------
  Reporter:  anonymous         |      Owner:  bemandawes
      Type:  Bugs              |     Status:  new
 Milestone:  To Be Determined  |  Component:  filesystem
   Version:  Boost 1.61.0      |   Severity:  Problem
Resolution:                    |   Keywords:
-------------------------------+------------------------

Comment (by mlimber@…):

 On further thought, perhaps the solution is that the
 {{{directory_iterator}}} should use some sort of observer pattern or weak
 reference rather than just a {{{shared_ptr}}} for its pointer to the
 implementation {{{m_imp}}}.

 At present, the iterator depends on {{{m_imp}}} becoming null when the
 last directory entry has been reached (see the comment in the
 operations.hpp: "m_imp.get()==0 indicates the end iterator."), but in the
 face of shared ownership, one owner can get to end without the others
 hearing about it or being able to detect it. From
 boost_1_62_0/libs/filesystem/operations.cpp:
 {{{#!cpp
   void directory_iterator_increment(directory_iterator& it,
     system::error_code* ec)
   {
       // ...
       if (it.m_imp->handle == 0)  // eof, make end
       {
         it.m_imp.reset();
         return;
       }
       // ...
   }
 }}}

 So the current copy of the {{{it}}} gets its {{{m_imp}}} reset and is thus
 now equal to the end iterator, but this does not tell the other shared
 owners of {{{m_imp}}} that they should reset too. By the nature of
 {{{shared_ptr}}}, the reference count decreased but the object still lives
 for all other owners.

 I personally would still prefer a copy-on-write or other deep-copy
 semantic (or making it a full-up forward iterator) but I understand that
 this may be difficult to impossible within the bounds of POSIX
 {{{readdir_r()}}} and other supported OS file systems.

 To sum up, possible paths I see to address this are:

 1. Make directory iterators do a deep copy or COW.
 1. Make directory iterators a {{{ForwardIterator}}} (additional semantics
 beyond deep copy).
 1. Make directory iterators move-only rather than copyable. This could
 involve changing the concept of {{{InputIterator}}} (or making a new
 concept {{{MoveableInputIterator}}} so as to leave other input iterators
 undisturbed).
 1. Make directory iterator use some sort of weak ref or observer pattern
 to update all copies that they have been invalidated.
 1. Update the documentation to indicate the pitfalls more clearly and
 suggest best practices for coding standards to adopt.

 I'm willing to lend a hand, but I don't want to go down a path that you
 all know would be rejected in principle. What is the best option?

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/12578#comment:10>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.