Skip to content

async_pipe: Refactor opened/closed state handling, avoid assign(-1) #190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dkl wants to merge 2 commits into from
Closed

async_pipe: Refactor opened/closed state handling, avoid assign(-1) #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e0ae20b
async_pipe: Refactor opened/closed state handling, avoid assign(-1)
dkl Dec 7, 2020
d5aab4d
posix pipes: Re-add strong exception guarantee for assignment operators
dkl Jan 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 26 additions & 77 deletions include/boost/process/detail/posix/async_pipe.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ class async_pipe
inline async_pipe(const async_pipe& lhs);
async_pipe(async_pipe&& lhs) : _source(std::move(lhs._source)), _sink(std::move(lhs._sink))
{
lhs._source.assign (-1);
lhs._sink .assign (-1);
}

template<class CharT, class Traits = std::char_traits<CharT>>
Expand Down Expand Up @@ -182,13 +180,11 @@ class async_pipe

handle_type source(::boost::asio::io_context& ios) const &
{
auto source_in = const_cast<::boost::asio::posix::stream_descriptor &>(_source).native_handle();
return ::boost::asio::posix::stream_descriptor(ios, ::dup(source_in));
return ::boost::asio::posix::stream_descriptor(ios, ::dup(native_source()));
}
handle_type sink (::boost::asio::io_context& ios) const &
{
auto sink_in = const_cast<::boost::asio::posix::stream_descriptor &>(_sink).native_handle();
return ::boost::asio::posix::stream_descriptor(ios, ::dup(sink_in));
return ::boost::asio::posix::stream_descriptor(ios, ::dup(native_sink()));
}
};

Expand All @@ -198,80 +194,43 @@ async_pipe::async_pipe(boost::asio::io_context & ios_source,
const std::string & name) : _source(ios_source), _sink(ios_sink)
{
auto fifo = mkfifo(name.c_str(), 0666 );

if (fifo != 0)
boost::process::detail::throw_last_error("mkfifo() failed");


int read_fd = open(name.c_str(), O_RDWR);

int read_fd = ::open(name.c_str(), O_RDWR);
if (read_fd == -1)
boost::process::detail::throw_last_error();
_source.assign(read_fd);

int write_fd = dup(read_fd);

int write_fd = ::dup(_source.native_handle());
if (write_fd == -1)
boost::process::detail::throw_last_error();

_source.assign(read_fd);
_sink .assign(write_fd);
_sink.assign(write_fd);
}

async_pipe::async_pipe(const async_pipe & p) :
_source(const_cast<async_pipe&>(p)._source.get_executor()),
_sink( const_cast<async_pipe&>(p)._sink.get_executor())
{

//cannot get the handle from a const object.
auto source_in = const_cast<::boost::asio::posix::stream_descriptor &>(_source).native_handle();
auto sink_in = const_cast<::boost::asio::posix::stream_descriptor &>(_sink).native_handle();
if (source_in == -1)
_source.assign(-1);
else
{
_source.assign(::dup(source_in));
if (_source.native_handle()== -1)
if (p._source.is_open()) {
int fd = ::dup(p.native_source());
if (fd == -1)
::boost::process::detail::throw_last_error("dup()");
_source.assign(fd);
}

if (sink_in == -1)
_sink.assign(-1);
else
{
_sink.assign(::dup(sink_in));
if (_sink.native_handle() == -1)
if (p._sink.is_open()) {
int fd = ::dup(p.native_sink());
if (fd == -1)
::boost::process::detail::throw_last_error("dup()");
_sink.assign(fd);
}
}

async_pipe& async_pipe::operator=(const async_pipe & p)
{
int source;
int sink;

//cannot get the handle from a const object.
auto source_in = const_cast<::boost::asio::posix::stream_descriptor &>(p._source).native_handle();
auto sink_in = const_cast<::boost::asio::posix::stream_descriptor &>(p._sink).native_handle();
if (source_in == -1)
source = -1;
else
{
source = ::dup(source_in);
if (source == -1)
::boost::process::detail::throw_last_error("dup()");
}

if (sink_in == -1)
sink = -1;
else
{
sink = ::dup(sink_in);
if (sink == -1)
::boost::process::detail::throw_last_error("dup()");
}
_source.assign(source);
_sink. assign(sink);

async_pipe copy{p};
std::swap(*this, copy);
return *this;
}

Expand All @@ -285,33 +244,23 @@ async_pipe& async_pipe::operator=(async_pipe && lhs)
template<class CharT, class Traits>
async_pipe::operator basic_pipe<CharT, Traits>() const
{
int source;
int sink;

//cannot get the handle from a const object.
auto source_in = const_cast<::boost::asio::posix::stream_descriptor &>(_source).native_handle();
auto sink_in = const_cast<::boost::asio::posix::stream_descriptor &>(_sink).native_handle();

basic_pipe<CharT, Traits> pipe{-1, -1};

if (source_in == -1)
source = -1;
else
{
source = ::dup(source_in);
if (source == -1)
if (_source.is_open()) {
int fd = ::dup(native_source());
if (fd == -1)
::boost::process::detail::throw_last_error("dup()");
pipe.assign_source(fd);
}

if (sink_in == -1)
sink = -1;
else
{
sink = ::dup(sink_in);
if (sink == -1)
if (_sink.is_open()) {
int fd = ::dup(native_sink());
if (fd == -1)
::boost::process::detail::throw_last_error("dup()");
pipe.assign_sink(fd);
}

return basic_pipe<CharT, Traits>{source, sink};
return pipe;
}


Expand Down
53 changes: 16 additions & 37 deletions include/boost/process/detail/posix/basic_pipe.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <unistd.h>
#include <fcntl.h>
#include <memory>
#include <utility>

namespace boost { namespace process { namespace detail { namespace posix {

Expand Down Expand Up @@ -48,10 +49,9 @@ class basic_pipe
}
inline basic_pipe(const basic_pipe& rhs);
explicit inline basic_pipe(const std::string& name);
basic_pipe(basic_pipe&& lhs) : _source(lhs._source), _sink(lhs._sink)
basic_pipe(basic_pipe&& lhs)
{
lhs._source = -1;
lhs._sink = -1;
*this = std::move(lhs);
}
inline basic_pipe& operator=(const basic_pipe& );
basic_pipe& operator=(basic_pipe&& lhs)
Expand All @@ -66,10 +66,7 @@ class basic_pipe
}
~basic_pipe()
{
if (_sink != -1)
::close(_sink);
if (_source != -1)
::close(_source);
close();
}
native_handle_type native_source() const {return _source;}
native_handle_type native_sink () const {return _sink;}
Expand Down Expand Up @@ -125,37 +122,23 @@ class basic_pipe
template<class CharT, class Traits>
basic_pipe<CharT, Traits>::basic_pipe(const basic_pipe & rhs)
{
if (rhs._source != -1)
{
_source = ::dup(rhs._source);
if (_source == -1)
::boost::process::detail::throw_last_error("dup() failed");
}
if (rhs._sink != -1)
{
if (rhs._source != -1) {
_source = ::dup(rhs._source);
if (_source == -1)
::boost::process::detail::throw_last_error("dup() failed");
}
if (rhs._sink != -1) {
_sink = ::dup(rhs._sink);
if (_sink == -1)
::boost::process::detail::throw_last_error("dup() failed");

}
}

template<class CharT, class Traits>
basic_pipe<CharT, Traits> &basic_pipe<CharT, Traits>::operator=(const basic_pipe & rhs)
{
if (rhs._source != -1)
{
_source = ::dup(rhs._source);
if (_source == -1)
::boost::process::detail::throw_last_error("dup() failed");
}
if (rhs._sink != -1)
{
_sink = ::dup(rhs._sink);
if (_sink == -1)
::boost::process::detail::throw_last_error("dup() failed");

}
basic_pipe<CharT, Traits> copy{rhs};
std::swap(*this, copy);
return *this;
}

Expand All @@ -164,23 +147,19 @@ template<class CharT, class Traits>
basic_pipe<CharT, Traits>::basic_pipe(const std::string & name)
{
auto fifo = mkfifo(name.c_str(), 0666 );

if (fifo != 0)
boost::process::detail::throw_last_error("mkfifo() failed");


int read_fd = open(name.c_str(), O_RDWR);

if (read_fd == -1)
boost::process::detail::throw_last_error();

int write_fd = dup(read_fd);

_source = read_fd;

int write_fd = dup(_source);
if (write_fd == -1)
boost::process::detail::throw_last_error();

_sink = write_fd;
_source = read_fd;

::unlink(name.c_str());
}

Expand Down
3 changes: 3 additions & 0 deletions test/async_pipe.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,11 @@ BOOST_AUTO_TEST_CASE(move_pipe)
asio::io_context ios;

bp::async_pipe ap{ios};
BOOST_CHECK(ap.is_open());
BOOST_TEST_CHECKPOINT("First move");
bp::async_pipe ap2{std::move(ap)};
BOOST_CHECK(!ap.is_open());
BOOST_CHECK(ap2.is_open());
#if defined(BOOST_WINDOWS_API)
BOOST_CHECK_EQUAL(ap.native_source(), ::boost::winapi::INVALID_HANDLE_VALUE_);
BOOST_CHECK_EQUAL(ap.native_sink (), ::boost::winapi::INVALID_HANDLE_VALUE_);
Expand Down