Re: Reminder and caution
Thanks for the explanation. Your OP said nothing about that, so was "hard to follow". On Wed, Jul 25, 2018 at 10:19 AM William A Rowe Jr wrote: > It seems we have a lot of revert activity on trunk, relating to OpenSSL > init, which is becoming hard to follow. It would be good if we start off > major changes on the dev@ list for discussion first, which should help > avoid some code churn. > > > > On Wed, Jul 25, 2018, 05:17 Greg Stein wrote: > >> What is this email about? >> >> >> On Tue, Jul 24, 2018 at 9:43 PM William A Rowe Jr >> wrote: >> >>> *Change Process* >>> >>> Most changes (bug fixes and minor, commonsense feature adds) do not >>> require review. Developers are encouraged to request review for: >>> >>>- Large changes affecting many files >>>- Changes to interfaces >>>- Changes that commit APR to one option out of an exclusive set >>>- Any change the developer wants a second (or Nth) opinion on >>> >>> Anyone may retroactively cause someone's change to be reviewed by >>> requesting review after it was committed, and at their discretion may >>> revert the change until it is approved. >>> >>> The above process is called "Commit Then Review" (or CTR). As of this >>> time, the group does not see a need to ever use a "Review Then Commit" >>> (RTC) process. >>> >>> >>>
Init/de-init of aprfoo-1.so providers
Giving this problem some thought... If an aprfoo-1.so provider is linked to the libfoo.so library, and we track the init state and perform it only once, it seems that apr should never try to de-init or unload the .so provider for the lifespan of the process. This should help us avoid some redundant initialization. While we could free resources atexit(), any spin of trying to repeatedly unload and reload any .so provider seems to be inherently messy and unstable. We see this issue with mod_ssl, etc, where the statics may be freed and reinitialized, and causes the resource leakage mentioned by Yann. Once loaded and initialized, we recieve no benefit from repeatedly altering the processes' library mmaps, and doing so after we spin up contending threads is an even worse scenario. Leaving them loaded should not have an adverse effect, and if it becomes swapped out due to lack of use during the lifespan of the process, should not be particularly harmful. While httpd modules are often unloaded and reloaded, the underlying libfoo.so is still stuck in process to libapr-1.so, so those should remain stable. Does this make sense?
Re: Reminder and caution
It seems we have a lot of revert activity on trunk, relating to OpenSSL init, which is becoming hard to follow. It would be good if we start off major changes on the dev@ list for discussion first, which should help avoid some code churn. On Wed, Jul 25, 2018, 05:17 Greg Stein wrote: > What is this email about? > > > On Tue, Jul 24, 2018 at 9:43 PM William A Rowe Jr > wrote: > >> *Change Process* >> >> Most changes (bug fixes and minor, commonsense feature adds) do not >> require review. Developers are encouraged to request review for: >> >>- Large changes affecting many files >>- Changes to interfaces >>- Changes that commit APR to one option out of an exclusive set >>- Any change the developer wants a second (or Nth) opinion on >> >> Anyone may retroactively cause someone's change to be reviewed by >> requesting review after it was committed, and at their discretion may >> revert the change until it is approved. >> >> The above process is called "Commit Then Review" (or CTR). As of this >> time, the group does not see a need to ever use a "Review Then Commit" >> (RTC) process. >> >> >>
Re: Request for review r1833421 (was: Reminder and caution)
On Wed, Jul 25, 2018 at 12:05 PM, Yann Ylavic wrote: > > Since I'm not really satisfied by the apr_crypto code in its > driver/DSO shape, I can as well ignore it and move out the PRNG code > to APR core (by copying a reference implementation of the needed > crypto primitive, from public domain, and quite simple btw). I probably missed the point here that r1833421 is not only about the PRNG needs, it's mainly about the APR providing (to the user) a way to initialize/terminate the crypto libraries used internally, but also possibly externally in multiple flavors.
Re: Reminder and caution
What is this email about? On Tue, Jul 24, 2018 at 9:43 PM William A Rowe Jr wrote: > *Change Process* > > Most changes (bug fixes and minor, commonsense feature adds) do not > require review. Developers are encouraged to request review for: > >- Large changes affecting many files >- Changes to interfaces >- Changes that commit APR to one option out of an exclusive set >- Any change the developer wants a second (or Nth) opinion on > > Anyone may retroactively cause someone's change to be reviewed by > requesting review after it was committed, and at their discretion may > revert the change until it is approved. > > The above process is called "Commit Then Review" (or CTR). As of this > time, the group does not see a need to ever use a "Review Then Commit" > (RTC) process. > > >
Request for review r1833421 (was: Reminder and caution)
On Wed, Jul 25, 2018 at 4:43 AM, Bill wrote: > > Change Process > > Most changes (bug fixes and minor, commonsense feature adds) do not > require review. Developers are encouraged to request review for: [] > - Changes to interfaces Changes that commit APR to one option out > of an exclusive set IIUC, my crypto libs' linking changes, needed IMO for any sane implementation of the new PRNG, would match this case. We are probably already above the "request for review" state since r1833421 was veto-ed, though I disagree with the arguments put forward. So could the next step be: > - Any change the developer wants a second (or Nth) opinion on or does the veto imply that I should revert the commit anyway, with no further discussion and/or others' opinions? Thanks for letting me know because I don't want to (and won't) dispute indefinitely for a lost cause... Since I'm not really satisfied by the apr_crypto code in its driver/DSO shape, I can as well ignore it and move out the PRNG code to APR core (by copying a reference implementation of the needed crypto primitive, from public domain, and quite simple btw). Regards, Yann.
