From ed906adc3597e1657481199bf6562c26f4805460 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Fri, 31 Mar 2017 11:15:27 -0400 Subject: [PATCH] Remove http Writer suspend and resume feature (API Change): fix #154 The resume_context parameter passed to instances of Writer during HTTP serialization is removed. --- CHANGELOG.md | 4 ++ doc/quickref.xml | 2 +- doc/types/Writer.qbk | 37 ++-------- examples/file_body.hpp | 21 +++--- include/beast/core/error.hpp | 8 +++ include/beast/http.hpp | 1 - include/beast/http/basic_dynabuf_body.hpp | 7 +- include/beast/http/concepts.hpp | 39 +--------- include/beast/http/empty_body.hpp | 7 +- include/beast/http/impl/write.ipp | 87 +++-------------------- include/beast/http/resume_context.hpp | 34 --------- include/beast/http/string_body.hpp | 7 +- test/Jamfile | 1 - test/http/CMakeLists.txt | 1 - test/http/resume_context.cpp | 9 --- test/http/write.cpp | 36 ++-------- 16 files changed, 53 insertions(+), 248 deletions(-) delete mode 100644 include/beast/http/resume_context.hpp delete mode 100644 test/http/resume_context.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index f478e850..558e89d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ * Add io_service completion invariants test +API Changes: + +* Remove http Writer suspend and resume feature + -------------------------------------------------------------------------------- 1.0.0-b31 diff --git a/doc/quickref.xml b/doc/quickref.xml index 96002191..874c5a69 100644 --- a/doc/quickref.xml +++ b/doc/quickref.xml @@ -42,7 +42,6 @@ request_header response response_header - resume_context streambuf_body string_body @@ -190,6 +189,7 @@ buffer_cat prepare_buffer prepare_buffers + system_category to_string write diff --git a/doc/types/Writer.qbk b/doc/types/Writer.qbk index 90d4e6e3..aef4601c 100644 --- a/doc/types/Writer.qbk +++ b/doc/types/Writer.qbk @@ -28,8 +28,6 @@ In this table: * `m` denotes a value of type `message const&` where `std::is_same:value == true`. -* `rc` is an object of type [link beast.ref.http__resume_context `resume_context`]. - * `ec` is a value of type [link beast.ref.error_code `error_code&`] * `wf` is a [*write function]: a function object of unspecified type provided @@ -72,8 +70,8 @@ In this table: ] ] [ - [`a.write(rc, ec, wf)`] - [`boost::tribool`] + [`a.write(ec, wf)`] + [`bool`] [ Called repeatedly after `init` succeeds. `wf` is a function object which takes as its single parameter any value meeting the requirements @@ -81,12 +79,7 @@ In this table: must remain valid until the next member function of `writer` is invoked (which may be the destructor). This function returns `true` to indicate all message body data has been written, or `false` if - there is more body data. If the return value is `boost::indeterminate`, - the implementation will suspend the operation until the writer invokes - `rc`. It is the writers responsibility when returning - `boost::indeterminate`, to acquire ownership of `rc` via move - construction and eventually call it or else undefined behavior - results. This function must be `noexcept`. + there is more body data. ] ] ] @@ -139,7 +132,6 @@ public: Postconditions: If return value is `true`: - * Callee does not take ownership of resume. * Callee made zero or one calls to `write`. * There is no more data remaining to write. @@ -147,18 +139,6 @@ public: * Callee does not take ownership of resume. * Callee made one call to `write`. - If return value is boost::indeterminate: - * Callee takes ownership of `resume`. - * Caller suspends the write operation - until `resume` is invoked. - - When the caller takes ownership of resume, the - asynchronous operation will not complete until the - caller destroys the object. - - @param resume A functor to call to resume the write operation - after the writer has returned boost::indeterminate. - @param ec Set to indicate an error. This will cause an asynchronous write operation to complete with the error. @@ -167,17 +147,12 @@ public: the writer must guarantee that the buffers remain valid until the next member function is invoked, which may be the destructor. - @return `true` if there is no more data to send, - `false` when there may be more data, - boost::indeterminate to suspend. - - @note Undefined behavior if the callee takes ownership - of resume but does not return boost::indeterminate. + @return `true` if there is no more data to send, + `false` when there may be more data. */ template - boost::tribool + bool write( - resume_context&&, error_code&, WriteFunction&& wf) noexcept; }; diff --git a/examples/file_body.hpp b/examples/file_body.hpp index 8aef3f2d..aa88ce2f 100644 --- a/examples/file_body.hpp +++ b/examples/file_body.hpp @@ -10,10 +10,8 @@ #include #include -#include #include #include -#include #include #include @@ -38,7 +36,8 @@ struct file_body writer& operator=(writer const&) = delete; template - writer(message const& m) noexcept + writer(message const& m) noexcept : path_(m.body) { } @@ -54,8 +53,8 @@ struct file_body { file_ = fopen(path_.c_str(), "rb"); if(! file_) - ec = boost::system::errc::make_error_code( - static_cast(errno)); + ec = error_code{errno, + system_category()}; else size_ = boost::filesystem::file_size(path_); } @@ -67,9 +66,8 @@ struct file_body } template - boost::tribool - write(resume_context&&, error_code&, - WriteFunction&& wf) noexcept + bool + write(error_code& ec, WriteFunction&& wf) noexcept { if(size_ - offset_ < sizeof(buf_)) buf_len_ = static_cast( @@ -77,7 +75,12 @@ struct file_body else buf_len_ = sizeof(buf_); auto const nread = fread(buf_, 1, sizeof(buf_), file_); - (void)nread; + if(ferror(file_)) + { + ec = error_code(errno, + system_category()); + return true; + } offset_ += buf_len_; wf(boost::asio::buffer(buf_, buf_len_)); return offset_ >= size_; diff --git a/include/beast/core/error.hpp b/include/beast/core/error.hpp index f94e52b5..42248b87 100644 --- a/include/beast/core/error.hpp +++ b/include/beast/core/error.hpp @@ -22,6 +22,14 @@ using system_error = boost::system::system_error; /// The type of error category used by the library using error_category = boost::system::error_category; +/// A function to return the system error category used by the library +#if GENERATING_DOCS +error_category const& +system_category(); +#else +using boost::system::system_category; +#endif + /// The type of error condition used by the library using error_condition = boost::system::error_condition; diff --git a/include/beast/http.hpp b/include/beast/http.hpp index dcb32791..a4bbfb52 100644 --- a/include/beast/http.hpp +++ b/include/beast/http.hpp @@ -19,7 +19,6 @@ #include #include #include -#include #include #include #include diff --git a/include/beast/http/basic_dynabuf_body.hpp b/include/beast/http/basic_dynabuf_body.hpp index cdae120a..d619cad6 100644 --- a/include/beast/http/basic_dynabuf_body.hpp +++ b/include/beast/http/basic_dynabuf_body.hpp @@ -10,10 +10,8 @@ #include #include -#include #include #include -#include namespace beast { namespace http { @@ -87,9 +85,8 @@ private: } template - boost::tribool - write(resume_context&&, error_code&, - WriteFunction&& wf) noexcept + bool + write(error_code&, WriteFunction&& wf) noexcept { wf(body_.data()); return true; diff --git a/include/beast/http/concepts.hpp b/include/beast/http/concepts.hpp index 9d43080c..27185d0d 100644 --- a/include/beast/http/concepts.hpp +++ b/include/beast/http/concepts.hpp @@ -10,9 +10,7 @@ #include #include -#include #include -#include #include #include @@ -50,38 +48,6 @@ struct has_content_length> -struct is_Writer : std::false_type {}; - -template -struct is_Writer().init( - std::declval()) - // VFALCO This is unfortunate, we have to provide the template - // argument type because this is not a deduced context? - // - ,std::declval().template write( - std::declval(), - std::declval(), - std::declval()) - )> > : std::integral_constant::value && - std::is_convertible().template write( - std::declval(), - std::declval(), - std::declval())), - boost::tribool>::value - > -{ - static_assert(std::is_same< - typename M::body_type::writer, T>::value, - "Mismatched writer and message"); -}; - -#else - template class is_Writer { @@ -99,10 +65,9 @@ class is_Writer template().template write( - std::declval(), std::declval(), std::declval())) - , boost::tribool>> + , bool>> static R check2(int); template static std::false_type check2(...); @@ -120,8 +85,6 @@ public: >; }; -#endif - template class is_Parser { diff --git a/include/beast/http/empty_body.hpp b/include/beast/http/empty_body.hpp index a1efddc2..9aa372b8 100644 --- a/include/beast/http/empty_body.hpp +++ b/include/beast/http/empty_body.hpp @@ -10,10 +10,8 @@ #include #include -#include #include #include -#include #include #include @@ -59,9 +57,8 @@ private: } template - boost::tribool - write(resume_context&&, error_code&, - WriteFunction&& wf) noexcept + bool + write(error_code&, WriteFunction&& wf) noexcept { wf(boost::asio::null_buffers{}); return true; diff --git a/include/beast/http/impl/write.ipp b/include/beast/http/impl/write.ipp index 7e97eebd..a6e64025 100644 --- a/include/beast/http/impl/write.ipp +++ b/include/beast/http/impl/write.ipp @@ -9,7 +9,6 @@ #define BEAST_HTTP_IMPL_WRITE_IPP #include -#include #include #include #include @@ -21,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -296,8 +294,6 @@ class write_op // VFALCO How do we use handler_alloc in write_preparation? write_preparation< isRequest, Body, Fields> wp; - resume_context resume; - resume_context copy; int state = 0; data(Handler& handler, Stream& s_, @@ -375,27 +371,9 @@ public: : d_(std::forward(h), s, std::forward(args)...) { - auto& d = *d_; - auto sp = d_; - d.resume = { - [sp]() mutable - { - write_op self{std::move(sp)}; - self.d_->cont = false; - auto& ios = self.d_->s.get_io_service(); - ios.dispatch(bind_handler(std::move(self), - error_code{}, 0, false)); - }}; - d.copy = d.resume; (*this)(error_code{}, 0, false); } - explicit - write_op(handler_ptr d) - : d_(std::move(d)) - { - } - void operator()(error_code ec, std::size_t bytes_transferred, bool again = true); @@ -460,8 +438,9 @@ operator()(error_code ec, std::size_t, bool again) case 1: { - boost::tribool const result = d.wp.w.write( - std::move(d.copy), ec, writef0_lambda{*this}); + auto const result = + d.wp.w.write(ec, + writef0_lambda{*this}); if(ec) { // call handler @@ -470,12 +449,6 @@ operator()(error_code ec, std::size_t, bool again) std::move(*this), ec, false)); return; } - if(boost::indeterminate(result)) - { - // suspend - d.copy = d.resume; - return; - } if(result) d.state = d.wp.chunked ? 4 : 5; else @@ -491,20 +464,15 @@ operator()(error_code ec, std::size_t, bool again) case 3: { - boost::tribool result = d.wp.w.write( - std::move(d.copy), ec, writef_lambda{*this}); + auto const result = + d.wp.w.write(ec, + writef_lambda{*this}); if(ec) { // call handler d.state = 99; break; } - if(boost::indeterminate(result)) - { - // suspend - d.copy = d.resume; - return; - } if(result) d.state = d.wp.chunked ? 4 : 5; else @@ -533,8 +501,6 @@ operator()(error_code ec, std::size_t, bool again) break; } } - d.copy = {}; - d.resume = {}; d_.invoke(ec); } @@ -640,37 +606,12 @@ write(SyncWriteStream& stream, wp.init(ec); if(ec) return; - std::mutex m; - std::condition_variable cv; - bool ready = false; - resume_context resume{ - [&] - { - std::lock_guard lock(m); - ready = true; - cv.notify_one(); - }}; - auto copy = resume; - boost::tribool result = - wp.w.write(std::move(copy), ec, - detail::writef0_lambda{stream, - wp.sb, wp.chunked, ec}); + auto result = wp.w.write( + ec, detail::writef0_lambda< + SyncWriteStream, decltype(wp.sb)>{ + stream, wp.sb, wp.chunked, ec}); if(ec) return; - if(boost::indeterminate(result)) - { - copy = resume; - { - std::unique_lock lock(m); - cv.wait(lock, [&]{ return ready; }); - ready = false; - } - boost::asio::write(stream, wp.sb.data(), ec); - if(ec) - return; - result = false; - } wp.sb.consume(wp.sb.size()); if(! result) { @@ -678,17 +619,11 @@ write(SyncWriteStream& stream, stream, wp.chunked, ec}; for(;;) { - result = wp.w.write(std::move(copy), ec, wf); + result = wp.w.write(ec, wf); if(ec) return; if(result) break; - if(! result) - continue; - copy = resume; - std::unique_lock lock(m); - cv.wait(lock, [&]{ return ready; }); - ready = false; } } if(wp.chunked) diff --git a/include/beast/http/resume_context.hpp b/include/beast/http/resume_context.hpp deleted file mode 100644 index 45da149b..00000000 --- a/include/beast/http/resume_context.hpp +++ /dev/null @@ -1,34 +0,0 @@ -// -// Copyright (c) 2013-2017 Vinnie Falco (vinnie dot falco at gmail dot com) -// -// Distributed under the Boost Software License, Version 1.0. (See accompanying -// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) -// - -#ifndef BEAST_HTTP_RESUME_CONTEXT_HPP -#define BEAST_HTTP_RESUME_CONTEXT_HPP - -#include - -namespace beast { -namespace http { - -/** A functor that resumes a write operation. - - An rvalue reference to an object of this type is provided by the - write implementation to the `writer` associated with the body of - a message being sent. - - If it is desired that the `writer` suspend the write operation (for - example, to wait until data is ready), it can take ownership of - the resume context using a move. Then, it returns `boost::indeterminate` - to indicate that the write operation should suspend. Later, the calling - code invokes the resume function and the write operation continues - from where it left off. -*/ -using resume_context = std::function; - -} // http -} // beast - -#endif diff --git a/include/beast/http/string_body.hpp b/include/beast/http/string_body.hpp index 9fbd1604..df110403 100644 --- a/include/beast/http/string_body.hpp +++ b/include/beast/http/string_body.hpp @@ -10,10 +10,8 @@ #include #include -#include #include #include -#include #include #include @@ -87,9 +85,8 @@ private: } template - boost::tribool - write(resume_context&&, error_code&, - WriteFunction&& wf) noexcept + bool + write(error_code&, WriteFunction&& wf) noexcept { wf(boost::asio::buffer(body_)); return true; diff --git a/test/Jamfile b/test/Jamfile index 6f275a39..52f7b69d 100644 --- a/test/Jamfile +++ b/test/Jamfile @@ -58,7 +58,6 @@ unit-test http-tests : http/parser_v1.cpp http/read.cpp http/reason.cpp - http/resume_context.cpp http/rfc7230.cpp http/streambuf_body.cpp http/string_body.cpp diff --git a/test/http/CMakeLists.txt b/test/http/CMakeLists.txt index a01c92cb..02e5db8b 100644 --- a/test/http/CMakeLists.txt +++ b/test/http/CMakeLists.txt @@ -23,7 +23,6 @@ add_executable (http-tests parser_v1.cpp read.cpp reason.cpp - resume_context.cpp rfc7230.cpp streambuf_body.cpp string_body.cpp diff --git a/test/http/resume_context.cpp b/test/http/resume_context.cpp deleted file mode 100644 index 0809b621..00000000 --- a/test/http/resume_context.cpp +++ /dev/null @@ -1,9 +0,0 @@ -// -// Copyright (c) 2013-2017 Vinnie Falco (vinnie dot falco at gmail dot com) -// -// Distributed under the Boost Software License, Version 1.0. (See accompanying -// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) -// - -// Test that header file is self-contained. -#include diff --git a/test/http/write.cpp b/test/http/write.cpp index 6aa84d3d..79fe11ab 100644 --- a/test/http/write.cpp +++ b/test/http/write.cpp @@ -55,9 +55,8 @@ public: } template - boost::tribool - write(resume_context&&, error_code&, - WriteFunction&& wf) noexcept + bool + write(error_code&, WriteFunction&& wf) noexcept { wf(boost::asio::buffer(body_)); return true; @@ -103,8 +102,6 @@ public: { std::size_t n_ = 0; value_type const& body_; - bool suspend_ = false; - enable_yield_to yt_; public: template @@ -120,37 +117,12 @@ public: body_.fc_.fail(ec); } - class do_resume - { - resume_context rc_; - - public: - explicit - do_resume(resume_context&& rc) - : rc_(std::move(rc)) - { - } - - void - operator()() - { - rc_(); - } - }; - template - boost::tribool - write(resume_context&& rc, error_code& ec, - WriteFunction&& wf) noexcept + bool + write(error_code& ec, WriteFunction&& wf) noexcept { if(body_.fc_.fail(ec)) return false; - suspend_ = ! suspend_; - if(suspend_) - { - yt_.get_io_service().post(do_resume{std::move(rc)}); - return boost::indeterminate; - } if(n_ >= body_.s_.size()) return true; wf(boost::asio::buffer(body_.s_.data() + n_, 1));