Subject: Re: [Boost-bugs] [Boost C++ Libraries] #1002: [iostreams] close_impl<closable_tag> does not comply with spec
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2007-12-03 02:28:01
#1002: [iostreams] close_impl<closable_tag> does not comply with spec
Reporter: chad_at_[hidden] | Owner: turkanis
Type: Bugs | Status: assigned
Milestone: | Component: iostreams
Version: | Severity: Problem
Resolution: | Keywords:
Changes (by turkanis):
* owner: dgregor => turkanis
* status: new => assigned
The following analysis was provide by Chad Walters. I am in the process of
implementing his proposal #3, below.
I have come to the conclusion that there may
need to be a somewhat larger set of changes than the patches that we have
been considering so far, at least for the long run. Let me walk through
reasoning and see if you agree.
Here are all the cases covered by the code in close.hpp, matched up
the semantics table and the rest of the close function documentation.
1. Non-closable devices and filters cannot end up in the closable_tag
specialization of close_impl. They don't get closed by the close function
they just get flushed.
2. Closable bidirectional devices and filters cannot end up in the
closable_tag specialization of close_impl -- they go to the two_sequence
version instead. They always get closed by the close function, regardless
the value of the which parameter, and invoke the device or filter's
3. Closable dual_use filters and details::teed_sequence devices and
follow the same path in the code as #2 above. These are not covered
explicitly in the documentation of the close function at all. I need to
understand the use of dual_use a little more to really understand this
I also note that teed_sequence appears only in tee.hpp and is actually
commented out in both cases. Furthermore, close.hpp is the only place in
library where teed_sequence is used. I think this notion is basically
deprecated at this point and should be removed.
4. A closable device or filter that is input only should be closed
to whether (which & ios_base::in) != 0, and invoke the close() method.
5. A closable device or filter that is output only should be closed
according to whether (which & ios_base::out) != 0, and invoke the close()
method. This is the case that I was trying to patch up -- the code was
the wrong thing here when (which & ios_base::out) != 0 && (which &
ios_base::in) != 0. The documentation under "When is close invoked?"
to some extent that the close function should only ever be invoked with
which parameter equal to exactly one of ios_base::in or ios_base::out, but
not both. But this case was happening nonetheless.
6. A closable device or filter can be input and output but not be
bidirectional (like the memory mapped file, as you point out). This means
that it controls a single sequence. The semantics table implies that this
sequence should get closed when (which & ios_base::out) != 0, just like
#4 above here. In addition, this would be consistent with the sequencing
the "When should close be invoked?" discussion for filtering_streambuf and
filtering_stream. The close function should get invoked twice for the
sequence device or filter, first with ios_base::in and then with
ios_base::out. Since this is a single sequence, it probably shouldn't be
closed until the second invocation (ie: the one with ios_base::out).
OK, let's pause here before pressing on... Hopefully everything so far
Based on the above, I think that the existing code in close.hpp would
basically have been correct if the which parameter could only be exactly
of ios_base::in or iosbase::out, but not both. As you pointed out, the
tee_filter and tee_device code can invoke the close function with the
parameter equal to (ios_base::in | ios_base::out). This conflict is the
direct source of the bug I reported.
I think that the true problem actually lies a little deeper. It seems to
that the fundamental issue stems from the fact that the Closable concept
requires that a Filter or Device provide different versions of close(),
depending on whether it controls a single sequence or two sequences (as
described under "Examples" in the Closable documentation): if they contain
single sequence, then they need to have a close method that doesn't have
ios_base::mode parameter; and if they contain two sequences, they must
a close method with an ios_base::mode parameter.
However, there are several devices and filters in the iostreams library
that, for simplicity's sake, provide a close method with an optional
value for the which parameter so that they can wrap other devices or
without caring about whether the wrapped device or filter has one or two
sequences. Furthermore, some of them, like tee_filter and tee_device, set
that which parameter to (ios_base::in | ios_base::out). This means that
close_impl can get called with this unexpected value.
There are 3 different possible avenues to fixing the problems with
with increasing levels of difficulty and architectural cleanliness.
1. Just make the spot fix for the particular bug that I encountered in
close.hpp; that is, make the close_impl for non-bidirectional closable
devices and filters behave as if (ios_base::in | ios_base::out) were a
input for the which parameter. I would adjust your patch by changing the
body of your should_close function: just remove the "if (hasIn && hasOut)
return false;" partl otherwise the wrong thing happens for case #6 above.
would also leave the documentation of the close function semantics
(ie: rollback the change you made) -- it is correctly stating the cases.
Also, no need to change the default parameter in tee_filter and tee_device
(in case something is done WRT issue #791).
2. Change all the devices and filters in the iostreams library that
a default value for the which parameter on their close methods to support
separate close methods, one without the which parameter and one with the
which parameter. Make sure the right version gets invoked by all the
in the library, depending on whether the underlying stream has one or two
sequences. There probably need to be specializations of tee_device and
tee_filter, depending on whether the underlying Devices have one or two
3. Do all the work for #2 above. In addition, change the Closable concept
and the close function so that only exactly two values for the which
parameter are possible. Simply changing to a bool would really be the
simplest option here, but I suppose other more complex template-based
could work as well.
Ordinarily, I'd recommend #1 for a short term fix (no API changes) for the
next bugfix release and #3 for the next major revision. However, we might
too close to 1.35.0 to do the changes for #3 in time. I guess #1 for
would work, with #3 for 1.36.0 in the (distant?) future would be the
Ticket URL: <http://svn.boost.org/trac/boost/ticket/1002#comment:4>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.
This archive was generated by hypermail 2.1.7 : 2017-02-16 18:49:57 UTC