On Mon, Jan 14, 2019 at 12:56 PM Stefan Sperling <s...@stsp.name> wrote:

> On Mon, Jan 14, 2019 at 11:38:55AM -0600, William A Rowe Jr wrote:
> > On Mon, Jan 14, 2019 at 8:42 AM Stefan Sperling <s...@stsp.name> wrote:
> > >
> > > FYI, the reason APR pool debugging is enabled in production on OpenBSD
> > > is because, after Heartbleed, OpenBSD decided to force 3rd party
> software
> > > to use the operating system's malloc/free implementation instead of
> custom
> > > allocators, where possible.
> > >
> > > If there was another way to make APR pools map directly to malloc/free
> > > I would like to know about it. As far as I undestand, APR pools will
> > > cache freed memory unless pool debugging is enabled.
> >
> > Your assessment is correct and by design. The argument by OpenBSD
> > makes as much sense as attempting to force C construction/destruction
> > on C++ sources, and OpenBSD users should expect side-effects of this
> > rather radical change. APR pool Debugging has never been tested for
> > the same level of robustness as the 'release' pool mechanics, nor have
> > the many applications which are built upon APR pools. OpenBSD needs
> > to consider their "port" of these many APR-consuming applications as
> > independently maintained.
> Well, I am quite happy with it as it is forcing me to fix bugs such as this
> one which nobody cared to fix for a decade (the quoted comment was written
> by gstein before SVN 1.0): https://svn.apache.org/r1850651

Yes, there are absolutely benefits to testing code under the AP_POOL_DEBUG
logic. My comments speak to the amount of testing and scrutiny that the
pool debug logic itself has been subjected to.

It seems that the mode was overloaded with too many meanings. If I had
to break them all out, they might look like;

Typical allocator / subpool default behavior
Implicit allocator per subpool

Typical allocator default reuse behavior
Free on pool release
Lock no-read/no-write on pool release (debugging: no address reuse)

And re-implement the previously broken apr_pool_lock() for read-only
access to write-once/nonvolatile-on-fork pools such as config data.

Then expand the API to permit some sort of apr_pool_split (_fork?)
as a complement to apr_pool_join. Expand the API to indicate the
handoff of the allocator from one thread to another or indicate that
the pool is free-threaded.

The apr_allocator_max_free_set() value of 0 was wrong. Zero should
always force no-reuse. -1 or similar should have been used for some
unlimited allocation reuse. A value of 1 today effectively does the same
thing as the correct implementation of 0.

Just some thoughts to start off discussion.

Reply via email to