Skip to content
This repository has been archived by the owner on Jun 11, 2022. It is now read-only.

Group process waiting on windows #67

Closed
andrejnau opened this issue Mar 10, 2017 · 13 comments
Closed

Group process waiting on windows #67

andrejnau opened this issue Mar 10, 2017 · 13 comments
Milestone

Comments

@andrejnau
Copy link
Contributor

andrejnau commented Mar 10, 2017

I wrote a few examples and checked on macOS 10.12.2 (clang-800.0.42.1) and Windows 10 (msvc 2017)

#include <boost/process.hpp>
#include <iostream>
#include <chrono>
#include <map>

namespace bp = boost::process;

int main()
{
	std::map<int, bp::child> q;
	bp::group g;

	for (int i = 1; i <= 5; ++i)
	{
#ifdef _WIN32
		std::string exe = "powershell";
		std::vector<std::string> args =
		{
			"-command",
			"Start-Sleep",
			"-s",
			std::to_string(i),
			";",
			"echo",
			std::to_string(i)
		};
#else
		std::string exe = "sleep";
		std::vector<std::string> args =
		{
			std::to_string(i),
			";",
			"echo",
			std::to_string(i)
		};
#endif
		q[i] = bp::child(
			bp::exe = exe,
			bp::args = args,
			bp::shell,
			g,
			bp::std_out > stdout);
	}

	while (!q.empty())
	{
		g.wait();
		for (auto it = q.begin(); it != q.end(); )
		{
			bp::child& c = it->second;
			if (!c.running())
			{
				std::cout << "exit " << it->first << std::endl;
				it = q.erase(it);
			}
			else
			{
				++it;
			}
		}
	}
}

This code works on macOS, but on Windows hangs on the line g.wait();

I rewrite this code without use bp::group

#include <boost/process.hpp>
#include <iostream>
#include <chrono>
#include <map>

namespace bp = boost::process;

int main()
{
	std::map<int, bp::child> q;

	for (int i = 1; i <= 5; ++i)
	{
#ifdef _WIN32
		std::string exe = "powershell";
		std::vector<std::string> args =
		{
			"-command",
			"Start-Sleep",
			"-s",
			std::to_string(i),
			";",
			"echo",
			std::to_string(i)
		};
#else
		std::string exe = "sleep";
		std::vector<std::string> args =
		{
			std::to_string(i),
			";",
			"echo",
			std::to_string(i)
		};
#endif
		q[i] = bp::child(
			bp::exe = exe,
			bp::args = args,
			bp::shell,
			bp::std_out > stdout);
	}

	while (!q.empty())
	{
		for (auto it = q.begin(); it != q.end(); )
		{
			bp::child& c = it->second;

			std::error_code ec;
			c.wait_for(std::chrono::milliseconds(100), ec);
			if (!c.running(ec))
			{
				std::cout << "exit " << it->first << std::endl;
				it = q.erase(it);
			}
			else
			{
				++it;
			}
		}
		std::this_thread::sleep_for(std::chrono::milliseconds(100));
	}
}

Now, i have a problem on macOS. I get following output:

exit 1
1
exit 2
exit 3
exit 4
2
exit 5
4
5

If I removec.wait_for(std::chrono::milliseconds(100), ec);
I will get the correct output:

1
exit 1
2
exit 2
3
exit 3
4
exit 4
5
exit 5

I think the problem is that _exit_status->store(exit_code) in wait_for it is called if boost::process::detail::api::wait_for ended with an error

@andrejnau
Copy link
Contributor Author

andrejnau commented Mar 10, 2017

Also this code

#include <boost/process.hpp>

namespace bp = boost::process;

int main()
{
	bp::child c = bp::child(
		"echo hello",
		bp::shell,
		bp::std_out > stdout);
	c.wait();
}

not work on macOS. I get following output /bin/sh: echo hello: command not found
But it work on Windows

@klemens-morgenstern
Copy link
Owner

The second issue should be fixed, it just worked for me. Which commit did you use?

@klemens-morgenstern
Copy link
Owner

I think the problem is that _exit_status->store(exit_code) in wait_for it is called if boost::process::detail::api::wait_for ended with an error

Hmm, that might be the case, but I'm not sure. Are you able to try that one, i.e. replace return !time_out_occured; with return ret != -1.
I am not sure why I use the timeout there, it looks like a bug. Does seem to be different on windows too.

@klemens-morgenstern
Copy link
Owner

The group.wait is in fact broken on windows. I will have to add a completion port for that which might take a while. I really do not have a test for that.

I'll let you know when I get around to working on that.

@andrejnau
Copy link
Contributor Author

The second issue should be fixed, it just worked for me. Which commit did you use?

I check on last commit. It work without use bp::shell, but with bp::shell i get /bin/sh: echo hello: command not found

@klemens-morgenstern
Copy link
Owner

Oh, I thought it failed to compile on windows, that's what I checked for.

@andrejnau
Copy link
Contributor Author

Are you able to try that one, i.e. replace return !time_out_occured; with return ret != -1.

Yes, it helps to solve the problem.

@klemens-morgenstern
Copy link
Owner

Alright, I'll get that one in and will mark group.wait* as broken for the 1.64 release.

@andrejnau
Copy link
Contributor Author

I check on last commit. It work without use bp::shell, but with bp::shell i get /bin/sh: echo hello: command not found

It was broken on commit c31f697, after this commit exe_cmd_init::cmd_shell generate incorrect command
/bin/sh -c "\"echo hello\""

@andrejnau
Copy link
Contributor Author

This code should threw the exception?

#include <boost/process.hpp>
#include <boost/process/extend.hpp>

namespace bp = boost::process;

int main()
{
	bp::child c = bp::child(
		"incorrect.exe",
		bp::extend::on_error([](auto&, const std::error_code& ec)
		{
			//...
		})
	);
	c.wait();
}

I expect that on_error callback is invoked, but throw exception.
Is this the correct behavior? I can pass in the constructor std::error_code. But it seems to me that if there is on_error callback, this is not entirely correct.

@klemens-morgenstern
Copy link
Owner

Alright, I'll check that the constructed command is done right. There was some issue with the quotes, I remember that.

@klemens-morgenstern
Copy link
Owner

Actually that should throw, the behaviour shuold only be influences by passing an std::error_code or ignore_error to the function.

@klemens-morgenstern klemens-morgenstern added this to the boost 1.65 milestone Apr 23, 2017
@klemens-morgenstern klemens-morgenstern changed the title How to wait for a group of processes Group process waiting on windows Apr 23, 2017
@klemens-morgenstern
Copy link
Owner

Finally done in e72127f, sorry for the long delay.

klemens-morgenstern added a commit that referenced this issue Jan 8, 2019
Use the non-deprecated ASIO APIs.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants