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!