Re: [boost] [system] virtual ~error_category

Subject: Re: [boost] [system] virtual ~error_category
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2018-02-12 19:45:42

On 12/02/2018 19:13, Peter Dimov via Boost wrote:
> The virtual destructor of error_category creates problems for
> constexpr-ification because it makes the class non-literal.
> 
> I can't think of a scenario in which we need the destructor to be
> virtual. It's probably virtual for "guideline" reasons - the class has
> virtual functions. But does it really need to be? Error categories
> aren't destroyed via a pointer to base. They are, in fact, static
> instances, not destroyed at all, except on program exit.
> 
> Can anyone think of a case in which error_category needs the virtual
> destructor?

The same idea (removing the virtual destructor) came up recently on SG14
for the proposed `status_domain` which is the proposed v2 of
`<system_error>`.

It's very valid for a custom error code category to allocate some sort
of internal state e.g. a hash table for looking up messages. This could
be allocated statically, but it might also be allocated by the category
instance on the basis that it was until now the same thing. This would
not be freed on destruction if the `error_category` destructor were not
virtual.

Some on SG14 felt "so what?" under the assumption that error categories
are surely never destroyed? But then I reminded people of dynamic
unloading of shared libraries.

We then came up with a solution for `status_domain` whereby its
`message()` is going to return an atomic reference counted holder which
is a shared_ptr in all but name (but not one of those). That way
lifetime can be tracked, and the "right thing (TM)" can be done on
dynamic shared library unload. We can also drop the virtual destructor,
and make the `status_domain` completely constexpr apart from the member
functions it has which can't be.

But as far as Boost.System goes, I think de-virtualising its destructor
is a major API break. I can see plenty of code out there in the wild
relying on that destructor firing when it's supposed to, and Boost
devirtualising it would equal a resource leak where there was none
before. That's as big as break as changing what `if(ec)` means (which
BTW is currently being deleted in `status_code` to force people to write
what they actually mean).

Niall

-- 
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/