I'm happy to do that, and to preserve your dir.c fix. Commit coming up, crossing fingers for a clean Linux maintainer compilation.
On Sun, Mar 31, 2019, 11:44 Yann Ylavic <ylavic....@gmail.com> wrote: > On Sun, Mar 31, 2019 at 5:29 PM William A Rowe Jr <wr...@rowe-clan.net> > wrote: > > > > On Sun, Mar 31, 2019, 09:15 Yann Ylavic <ylavic....@gmail.com> wrote: > >> > >> On Sun, Mar 31, 2019 at 3:58 AM William A Rowe Jr <wr...@rowe-clan.net> > wrote: > >> > > >> > Let me just throw this out there and get your reaction, Yann, now that > >> > I'm back in town... The patch I was initially using to accomplish the > >> > equivalent was as simple as; > >> [] > >> > Seems much less wordy for a feature we agree should just be dropped > >> > altogether fairly soon. Thoughts? > >> > >> I don't know actually, I think it's not really "fair" to say that > >> readdir() is thread-safe, you can't really call it from different > >> threads with the same DIR arg, while you might with readdir_r() > >> (besides all its caveats). I'm talking about the system functions > >> here, not apr_dir_read() which is not thread-safe (on the same > >> apr_dir_t) with either underlying function, as we discussed elsewhere. > >> But who would do that anyway?! I think we should do[x]cument that just > in case. > >> > >> As for your patch vs mine, it depends on whether it exists systems > >> (that we still support) on which readdir() is really not thread-safe > >> even with different passed-in DIRs (e.g. the returned struct dirent is > >> static somewhere in the implementation, as opposed to some room > >> reserved in the struct DIR itself like in "modern" implementations), > >> and these systems provide readdir_r() for the "thread-safe" way. > >> I suppose such system wouldn't emit a warning for readdir_r(), my > >> patch addresses them while yours does not. > >> So the question is, does such system exists and do we want to still > support it? > > > > > > I think our answer is we never did support this. > > > > My thought is that all support this, inasmuch as they support concurrent > dlopen's and readdir's on the same thread. If they are truly not thread > safe (as opposed to parallel or reentrant) on different handles that would > be a very thread-unsafe architecture in the first place, no? A perfect > example for APR has no threads at all. > > > > Since our readdir is threadsafe flag has a well established meaning > (thread safe for APR's purpose alone) and we can preserve that meaning with > no harm between 1.0 and 1.x builds (all overrides preserve the desired > behavior) I'm in favor of the simplest solution. > > > > I'm entirely in favor of dropping the macro from 2.0 and never returning > to dirread_r() after all the explanatory docs and your evaluation of APR. > If user wants to walk a single directory across parallel threads, they > clearly need to serialize the actual directory read and use a serialized > allocator in that odd edge case, much as they must do the same for any sane > file read. > > Make sense, I'm fine with this. > If you want to proceed by reverting my related changes, please go down > to r1789258 since this first commit was an (unsatisfactory) try later > overwritten by r1856189. The comment change from r1789258 might be > preserved though. > Thanks for the simplification! >
