Re: [Boost-bugs] [Boost C++ Libraries] #12542: multi_index constraint violation if rollback callable is passed, but doesn't correct the issue

Subject: Re: [Boost-bugs] [Boost C++ Libraries] #12542: multi_index constraint violation if rollback callable is passed, but doesn't correct the issue
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2016-10-22 14:51:12

#12542: multi_index constraint violation if rollback callable is passed, but
doesn't correct the issue
-----------------------------------+-------------------------
  Reporter:  Jon Kalb <jonkalb@…>  |      Owner:  joaquin
      Type:  Bugs                  |     Status:  new
 Milestone:  To Be Determined      |  Component:  multi_index
   Version:  Boost 1.61.0          |   Severity:  Problem
Resolution:                        |   Keywords:
-----------------------------------+-------------------------

Comment (by Jon Kalb <jonkalb@…>):

 Joaquin,

 Thanks very much for your library and for your quick response on this
 issue.

 We'll just have to disagree on the correct design for this feature.

 Obviously I believe that the design is so dangerous that I reported it as
 a bug rather than a design flaw. It never occurred to me that this design
 was intentional.

 The possibility of error on the user's part, particularly in a non-trivial
 situation, the slight additional overhead of the verification
 (particularly given the likely uncommonness of the rollback needing to be
 called), and the catastrophic nature of the potential failure are what
 make me think that shifting the burden to the user here is ill-designed.

 Consider that the user cannot verify that fallback either will or has done
 the correct thing, nor is there a way to query if the index has been
 corrupted (not that there would be a way to correct the situation if it
 were).

 I discovered this while preparing a talk on various Boost libraries. When
 presenting this I will warn attendees that the rollback feature is unsafe
 to use in any but the most trivial cases. This warning will certainly
 weaken the generally positive thrust of the talk on this library, but to
 omit the warning would be a disservice to my audience.

 Of course if it were possible for the user to themselves verify (in the
 rollback) if the rollback has met the requirement, then it would be a
 usable feature. In that case, the best practice recommendation would be
 for the user to *always* do the verify in any non-trivial situations
 (where a uniqueness constraint is or may be in place). So the user is not
 benefiting from the lack of "uncalled for overhead," because the best
 practice would be to call for it. (Of course an opt-in method of disabling
 the check would be a useful feature in the, uncommon, I would expect,
 situation where the rollback is both in a tight loop and guaranteed to be
 successful.)

 Just rhetorically I'll ask you:  Why did you make all iterators const?
 This design decision adds an uncalled for overhead in those situations
 where the user wishes to modify data that the users knows is not in any
 index. But it is clearly the correct decision because otherwise the burden
 placed on the user and the devastating nature of a mistake on the user's
 part would make this a dangerous library to use.

 As it is, it has one very dangerous design flaw--the rollback feature.

 I didn't address the invariant-checking mode issue because I feel that is
 clearly not relevant. It effects performance of the entire library, not
 just in the probably rare rollback scenario and it isn't well designed to
 be useful in that scenario.

 But I do agree very much with one thing in the documentation describing
 the invariant-checking mode: "Any assertion of this kind should be
 regarded in principle as a bug in the library." I think that any
 opportunity for the user to corrupt the index in "normal" code (code
 without casts, buffer overruns, or language undefined behavior) is a bug
 in the design of the library.

 Please reconsider the design of this particular feature.

 Thanks again for your library and for your quick reply.

 Jon

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/12542#comment:2>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.