Here are the results of the Boost.Process project within this year's Google Summer of Code program: We have a complete library to manage system processes now! If you like to play around with the library download the ZIP file from <http://www.highscore.de/boost/gsoc2010/process.zip>. As it's a header-only library simply copy the files to your Boost folder. If you like to quickly check the documentation click on <http://www.highscore.de/boost/gsoc2010/>. The library is now complete and basically ready for a review. As there have been so many Boost.Process drafts before (after all the first version was created 2006) this library's version number is 0.4. This will somehow make it possible to refer to the various drafts if it's necessary. For now I would appreciate if the library's test cases are run on as many platforms as possible. After copying the files from the ZIP file to the Boost folder change to libs/process/test/ and run bjam - that's all! What has been changed? * All previous Boost.Process drafts including Ilya's 2008 version from the vault have been thoroughly studied to create a better version. From the multiple Boost.Process drafts which have been floating around since 2006 we took the best ideas and created a new unified version. * The design and implementation of the library have been simplified. The library's API is smaller, and the implementation is easier to understand. It should be possible to benefit from this library as quickly as possible. Even developers who have no experience in managing system processes should be able to do so easily. * There are no POSIX and Windows specific classes anymore (posix_context, posix_child, win32_context, win32_child etc. have been dropped). Instead of trying to support each and every feature platforms provide extension points have been defined. They enable library users to "link in" those platform specific features they are interested in. * For the first time there is full support for synchronous and asynchronous operations. While asynchronous operations are built on top of Boost.Asio synchronous operations can be utilized without that library. Developers don't need to include any Boost.Asio headers and don't need to learn about Boost.Asio if they don't need asynchronous operations. * Stream behaviors define if and how standard streams can be used by a child process. While the various stream behaviors were defined as an enumeration or a boost::variant in previous Boost.Process drafts they are now implemented as classes. This makes it possible for developers to create new stream behaviors. * The code to start child processes on POSIX platforms has been carefully adapted to support multi-threaded applications (it turned out that this code was wrong in all Boost.Process drafts so far). The documentation explains what you need to take care of if you want to create child processes in a multi-threaded application on POSIX platforms. * The documentation is new and has been shortened and simplified. There is a user guide which can be read in 10 minutes and introduces all the major features of the library. The documentation is now also based on Quickbook. * The test cases have been restructured to simplify them. They can now be run without patching Boost.Test and without setting a path to a helper executable in the source files - it's now all automatic. They are all passed successfully with MSVC 9 (2008) and g++ 4.2.1. What is currently missing? * Pipelines are not (yet) supported. If you have been using boost::process::launch_pipeline() there is nothing in this Boost.Process draft you can use instead. Depending on feedback this feature will be added again (there was simply no time yet to do this). We hope that after four years, two Google Summer of Code programs and countless drafts this new version will finally become the official Boost.Process library so many have been waiting for. Boris and Felipe
15/08/2010 13:03, Boris Schaeling wrote:
Here are the results of the Boost.Process project within this year's Google Summer of Code program: We have a complete library to manage system processes now!
Well done! Looks quite easy to use.
If you like to play around with the library download the ZIP file from <http://www.highscore.de/boost/gsoc2010/process.zip>. As it's a header-only library simply copy the files to your Boost folder. If you like to quickly check the documentation click on <http://www.highscore.de/boost/gsoc2010/>.
A few documentation problems: - detail headers shouldn't appear - pistream/postream do not inherit from std::istream/std::ostream nor to have any stream-like members, at least according to the reference - the Asynchronous I/O section should clearly say you need to use named pipes for this to work.
On Sun, 15 Aug 2010 15:06:58 +0200, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
[...]A few documentation problems: - detail headers shouldn't appear
Jamfile updated.
- pistream/postream do not inherit from std::istream/std::ostream nor to have any stream-like members, at least according to the reference
Hm, indeed! I think the problem is that Doxygen doesn't know std::istream and std:
- the Asynchronous I/O section should clearly say you need to use named pipes for this to work.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Sun, 15 Aug 2010 15:06:58 +0200, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
[...]A few documentation problems: - detail headers shouldn't appear
Jamfile updated.
- pistream/postream do not inherit from std::istream/std::ostream nor to have any stream-like members, at least according to the reference
Hm, indeed! I think the problem is that Doxygen doesn't know std::istream and std::ostream. I don't know if there is anything which can be done about it?
- the Asynchronous I/O section should clearly say you need to use named pipes for this to work.
It depends on the platform you are looking from: For POSIX Windows is the exception and the only platform which requires named pipes. For Windows POSIX is the exception and the only platform which doesn't require named pipes. :-) Thanks for your feedback, Boris
On 15.08.2010 17:48, Boris Schaeling wrote:
On Sun, 15 Aug 2010 15:06:58 +0200, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
[...]A few documentation problems: [snip] - the Asynchronous I/O section should clearly say you need to use named pipes for this to work.
It depends on the platform you are looking from: For POSIX Windows is the exception and the only platform which requires named pipes. For Windows POSIX is the exception and the only platform which doesn't require named pipes. :-)
Could we use named pipes on Windows for both sync and async IO?
On 08/15/2010 02:03 PM, Boris Schaeling wrote:
Here are the results of the Boost.Process project within this year's Google Summer of Code program: We have a complete library to manage system processes now!
Just looked briefly over the documentation. Very nice indeed! Using asio for communication with the child process sounds interesting. I pretty sure that Boost.Process will be used in our next maintenance phase once it is part of Boost. Until then, our home-grown stuff will have to remain in place... Can you give me a hint what the problem was with multi-threading in previous versions? Regards, Roland
On Sun, 15 Aug 2010 17:23:42 +0200, Roland Bock <rbock@eudoxos.de> wrote:
[...]Can you give me a hint what the problem was with multi-threading in previous versions?
See <http://www.opengroup.org/onlinepubs/000095399/functions/fork.html>: "A process shall be created with a single thread. If a multi-threaded process calls fork(), the new process shall contain a replica of the calling thread and its entire address space, possibly including the states of mutexes and other resources. Consequently, to avoid errors, the child process may only execute async-signal-safe operations until such time as one of the exec functions is called." All previous versions of Boost.Process had called non-async-signal-safe functions between fork() and exec(). Boris
On 08/15/2010 05:34 PM, Boris Schaeling wrote:
On Sun, 15 Aug 2010 17:23:42 +0200, Roland Bock <rbock@eudoxos.de> wrote:
[...]Can you give me a hint what the problem was with multi-threading in previous versions?
See <http://www.opengroup.org/onlinepubs/000095399/functions/fork.html>:
"A process shall be created with a single thread. If a multi-threaded process calls fork(), the new process shall contain a replica of the calling thread and its entire address space, possibly including the states of mutexes and other resources. Consequently, to avoid errors, the child process may only execute async-signal-safe operations until such time as one of the exec functions is called."
All previous versions of Boost.Process had called non-async-signal-safe functions between fork() and exec().
Thanks! Phew! Our child code just uses close and dup2 between fork and exec. Should be ok then, but this is the kind of stuff that makes me eager to see Boost.Progress reviewed and included :-)
On Tue, 17 Aug 2010 16:21:47 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
Boris Schaeling wrote:
* The code to start child processes on POSIX platforms has been carefully adapted to support multi-threaded applications
You still have a bug here. std::string::c_str() (at boost/process/operations.hpp:221) is not async-signal-safe.
Thanks, fixed! By the way, I'd appreciate if you could have a look at the overall design. I'm not sure if you lost interested in Boost.Process or you were simply busy elsewhere in 2008. Any comments from developers who played around with process management a bit more definitely help of course! Boris
* There are no POSIX and Windows specific classes anymore (posix_context, posix_child, win32_context, win32_child etc. have been dropped). Instead of trying to support each and every feature platforms provide extension points have been defined. They enable library users to "link in" those platform specific features they are interested in.
Excellent! I was hoping that the basic functionality would be platform-independent. Now it is (mostly). Is there a way to further remove platform dependence in handling the "exit code"?
* For the first time there is full support for synchronous and asynchronous operations. While asynchronous operations are built on top of Boost.Asio synchronous operations can be utilized without that library. Developers don't need to include any Boost.Asio headers and don't need to learn about Boost.Asio if they don't need asynchronous operations.
Interesting. Just out of curiosity, how did you implement asynchronous pipes on Windows? Reads and writes from/to anonymous pipes that are created with CreatePipe are blocking.
* Stream behaviors define if and how standard streams can be used by a child process. While the various stream behaviors were defined as an enumeration or a boost::variant in previous Boost.Process drafts they are now implemented as classes. This makes it possible for developers to create new stream behaviors.
* The code to start child processes on POSIX platforms has been carefully adapted to support multi-threaded applications (it turned out that this code was wrong in all Boost.Process drafts so far). The documentation explains what you need to take care of if you want to create child processes in a multi-threaded application on POSIX platforms.
* The documentation is new and has been shortened and simplified. There is a user guide which can be read in 10 minutes and introduces all the major features of the library. The documentation is now also based on Quickbook.
The documentation looks good, and I like that it is based on Quickbook.
* The test cases have been restructured to simplify them. They can now be run without patching Boost.Test and without setting a path to a helper executable in the source files - it's now all automatic. They are all passed successfully with MSVC 9 (2008) and g++ 4.2.1.
What is currently missing?
* Pipelines are not (yet) supported. If you have been using boost::process::launch_pipeline() there is nothing in this Boost.Process draft you can use instead. Depending on feedback this feature will be added again (there was simply no time yet to do this).
We hope that after four years, two Google Summer of Code programs and countless drafts this new version will finally become the official Boost.Process library so many have been waiting for.
Boris and Felipe
Here are some of my wish list items: * Templating of the char type to support wchar_t. The "wchar_t version" would use the wide versions of Windows functions: CreateProcessW, etc. * The ability to allocate within another process' address space, as well as read/write from/to another process' memory. * Integration with Boost Filesystem Also, two questions: * In the example that uses <goog_1014815798>async_wait<http://www.highscore.de/boost/gsoc2010/process/user_guide.html#boost_process.user_guide.waiting>, what happens if the child process exits before async_wait is called, and before the status object is created? Will the end_wait function still be called? * Do I need to call wait or async_wait to prevent the accumulation of zombie processes?
On Tue, 17 Aug 2010 19:59:43 +0200, Daniel Trebbien <dtrebbien@gmail.com> wrote:
[...]Excellent! I was hoping that the basic functionality would be platform-independent. Now it is (mostly).
Is there a way to further remove platform dependence in handling the "exit code"?
You mean the WIFEXITED et al macros? We had a status class in previous Boost.Process versions which encapsulated the exit code. However the design of the class was not really cross-platform as member functions like exited() were clearly added for POSIX systems only and didn't make sense on Windows (exited() for example always returned true). Thus the question is what to make platform-independent if a concept doesn't exist on a platform. :-)
[...]Interesting. Just out of curiosity, how did you implement asynchronous pipes on Windows? Reads and writes from/to anonymous pipes that are created with CreatePipe are blocking.
Yes, exactly! That's why named pipes must be used on Windows for asynchronous I/O (a behavior class called named_pipe is provided by the library).
[...]Here are some of my wish list items:
* Templating of the char type to support wchar_t. The "wchar_t version" would use the wide versions of Windows functions: CreateProcessW, etc.
The problem is then what to do on POSIX platforms (as there is no wide version of execve() for example)?
* The ability to allocate within another process' address space, as well as read/write from/to another process' memory.
I think you are looking for Boost.Interprocess. :-)
* Integration with Boost Filesystem
Where would you like to see support? Passing a path object to create_child() for example?
Also, two questions:
* In the example that uses <goog_1014815798>async_wait<http://www.highscore.de/boost/gsoc2010/process/user_guide.html#boost_process.user_guide.waiting>, what happens if the child process exits before async_wait is called, and before the status object is created? Will the end_wait function still be called?
Yes.
* Do I need to call wait or async_wait to prevent the accumulation of zombie processes?
Yes. Thanks for your feedback, Boris
Is there a way to further remove platform dependence in handling the "exit code"?
You mean the WIFEXITED et al macros? We had a status class in previous Boost.Process versions which encapsulated the exit code. However the design of the class was not really cross-platform as member functions like exited() were clearly added for POSIX systems only and didn't make sense on Windows (exited() for example always returned true). Thus the question is what to make platform-independent if a concept doesn't exist on a platform. :-)
A while ago when I was thinking about this same problem, I considered using Windows' debugging mechanism (process creation flag DEBUG_ONLY_THIS_PROCESS) to obtain this information. I don't know if debugging a process will affect its execution in some way, or whether the performance will be substantially impacted, but it might be interesting to see if this approach would work.
* Templating of the char type to support wchar_t. The "wchar_t version" would use the wide versions of Windows functions: CreateProcessW, etc.
The problem is then what to do on POSIX platforms (as there is no wide version of execve() for example)?
While there is no POSIX equivalent of the wide char functions, I was mainly thinking about using the `TCHAR` macro in cross-platform builds. Windows programmers will probably be programming with TCHAR set to `wchar_t`, so if the process creation library supported templating of the char type, then there wouldn't need to be conversion of wide char strings to narrow char strings. Plus, wouldn't there be a problem trying to execute a module on Windows that has a non-Latin character in it?
* The ability to allocate within another process' address space, as well as read/write from/to another process' memory.
I think you are looking for Boost.Interprocess. :-)
I read that the interprocess allocators of Boost.Interprocess require a pre-existing shared memory segment. I was thinking about cross-platform versions of Windows' VirtualAllocEx function<http://msdn.microsoft.com/en-us/library/aa366890.aspx>which allows blocks of memory to be allocated directly within another process' address space, and ReadProcessMemory<http://msdn.microsoft.com/en-us/library/ms680553.aspx>/ WriteProcessMemory <http://msdn.microsoft.com/en-us/library/ms681674.aspx>for accessing the memory.
* Integration with Boost Filesystem
Where would you like to see support? Passing a path object to create_child() for example?
For some reason it looked strange to me that the find_executable_in_pathfunction returned a string rather than a Boost Filesystem path object. It's not that big of a deal, though, because I could construct a path from a string if I really wanted.
On Wed, 18 Aug 2010 16:18:56 -0700, Daniel Trebbien wrote:
wouldn't there be a problem trying to execute a module on Windows that has a non-Latin character in it?
Yes. Boost.Process uses CreateProcessA so would fail to run any executable whose name or path contains a character not present in the user's current ANSI codepage. This means a Russian user could execute a Russian exe but a Russian user could not execute, say, an Arabic one. A British user could execute neither.
* Integration with Boost Filesystem
Where would you like to see support? Passing a path object to create_child() for example?
For some reason it looked strange to me that the find_executable_in_pathfunction returned a string rather than a Boost Filesystem path object. It's not that big of a deal, though, because I could construct a path from a string if I really wanted.
Ideally I would also love to see Boost paths used wherever a string is really a path but I can understand if the reason for not doing this is to avoid a dependecy on Boost.Path. Alex -- Swish - Easy SFTP for Windows Explorer (http://www.swish-sftp.org)
Le 19/08/2010 16:40, Alexander Lamaison a écrit :
On Wed, 18 Aug 2010 16:18:56 -0700, Daniel Trebbien wrote:
wouldn't there be a problem trying to execute a module on Windows that has a non-Latin character in it?
Yes. Boost.Process uses CreateProcessA so would fail to run any executable whose name or path contains a character not present in the user's current ANSI codepage. This means a Russian user could execute a Russian exe but a Russian user could not execute, say, an Arabic one. A British user could execute neither.
How about taking UTF-8 strings and converting them to whatever is best for the operating system?
Mathias Gaunard wrote:
Le 19/08/2010 16:40, Alexander Lamaison a écrit :
On Wed, 18 Aug 2010 16:18:56 -0700, Daniel Trebbien wrote:
wouldn't there be a problem trying to execute a module on Windows that has a non-Latin character in it?
Yes. Boost.Process uses CreateProcessA so would fail to run any executable whose name or path contains a character not present in the user's current ANSI codepage. This means a Russian user could execute a Russian exe but a Russian user could not execute, say, an Arabic one. A British user could execute neither.
How about taking UTF-8 strings and converting them to whatever is best for the operating system?
+1
On Fri, 20 Aug 2010 00:18:19 +0400, Ilya Sokolov wrote:
Mathias Gaunard wrote:
Le 19/08/2010 16:40, Alexander Lamaison a écrit :
On Wed, 18 Aug 2010 16:18:56 -0700, Daniel Trebbien wrote:
wouldn't there be a problem trying to execute a module on Windows that has a non-Latin character in it?
Yes. Boost.Process uses CreateProcessA so would fail to run any executable whose name or path contains a character not present in the user's current ANSI codepage.
How about taking UTF-8 strings and converting them to whatever is best for the operating system?
+1
It's an option (one I like), but is inconsistent with the way other APIs work on Windows and, for various reasons that Artyom can probably explain better than I, is tricky to implement on Windows. There was a thread a while back about getting Boost.Filesystem to treat narrow paths this way. I don't entirely recall the outcome. Alex -- Swish - Easy SFTP for Windows Explorer (http://www.swish-sftp.org)
On Thu, 19 Aug 2010 01:18:56 +0200, Daniel Trebbien <dtrebbien@gmail.com> wrote:
[...]While there is no POSIX equivalent of the wide char functions, I was mainly thinking about using the `TCHAR` macro in cross-platform builds. Windows programmers will probably be programming with TCHAR set to `wchar_t`, so if the process creation library supported templating of the char type, then there wouldn't need to be conversion of wide char strings to narrow char strings. Plus, wouldn't there be a problem trying to execute a module on Windows that has a non-Latin character in it?
Ok, I'll look into it! I don't remember anymore all the details why I gave up quickly in previous versions. As the implementation has been simplified maybe it's easier now to support wide chars somehow.
[...]I read that the interprocess allocators of Boost.Interprocess require a pre-existing shared memory segment. I was thinking about cross-platform versions of Windows' VirtualAllocEx function<http://msdn.microsoft.com/en-us/library/aa366890.aspx>which allows blocks of memory to be allocated directly within another process' address space, and ReadProcessMemory<http://msdn.microsoft.com/en-us/library/ms680553.aspx>/ WriteProcessMemory <http://msdn.microsoft.com/en-us/library/ms681674.aspx>for accessing the memory.
Ah, I see! I'd say this belongs to Boost.Interprocess. ;-) I'll probably not work on this anytime soon. In general I'm glad that after four years Boost.Process is so complete that I would dare to ask for a review. What Felipe and I had actually also planned was adding a create_snapshot() function and a parent class. So far Boost.Process is most useful when managing child processes. With create_snapshot() and parent it would also provide support to access non-related and parent processes. This would make the library a bit more complete (after all it's called Boost.Process and not Boost.ChildProcess :-). If anyone wants to contribute some features though I'd definitely help to add them to the library!
[...]For some reason it looked strange to me that the find_executable_in_pathfunction returned a string rather than a Boost Filesystem path object. It's not that big of a deal, though, because I could construct a path from a string if I really wanted.
You are right, path makes sense here. I can change this if it is preferred? Boris
Boris Schaeling wrote:
On Thu, 19 Aug 2010 01:18:56 +0200, Daniel Trebbien <dtrebbien@gmail.com> wrote:
...
[...]For some reason it looked strange to me that the find_executable_in_pathfunction returned a string rather than a Boost Filesystem path object. It's not that big of a deal, though, because I could construct a path from a string if I really wanted.
You are right, path makes sense here. I can change this if it is preferred?
+1 Jeff
Boris Schaeling wrote:
On Thu, 19 Aug 2010 01:18:56 +0200, Daniel Trebbien <dtrebbien@gmail.com> wrote:
[...]While there is no POSIX equivalent of the wide char functions, I was mainly thinking about using the `TCHAR` macro in cross-platform builds. Windows programmers will probably be programming with TCHAR set to `wchar_t`, so if the process creation library supported templating of the char type, then there wouldn't need to be conversion of wide char strings to narrow char strings. Plus, wouldn't there be a problem trying to execute a module on Windows that has a non-Latin character in it?
Ok, I'll look into it! I don't remember anymore all the details why I gave up quickly in previous versions. As the implementation has been simplified maybe it's easier now to support wide chars somehow.
I think we should wait for encoding-aware string class. It will allow to solve most of i18n issues in many libraries in a uniform way. The libraries I mean are Boost.Asio, Boost.Interprocess, Boost.System, Boost.Process and so on.
[snip]
[...]For some reason it looked strange to me that the find_executable_in_pathfunction returned a string rather than a Boost Filesystem path object. It's not that big of a deal, though, because I could construct a path from a string if I really wanted.
You are right, path makes sense here. I can change this if it is preferred?
-1. If you change find_executable_in_path() to return boost::filesystem?some?version?::path, then create_child(std::string, ) wouldn't accept the return value. If you change create_child() to accept boost::fs::path, then what type context::process_name, context::work_dir and others would have? The last but not least, Boost.Filesystem is not header-only.
On Thu, 19 Aug 2010 22:13:57 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
Boris Schaeling wrote:
On Thu, 19 Aug 2010 01:18:56 +0200, Daniel Trebbien <dtrebbien@gmail.com> wrote:
[...]While there is no POSIX equivalent of the wide char functions, I was mainly thinking about using the `TCHAR` macro in cross-platform builds. Windows programmers will probably be programming with TCHAR set to `wchar_t`, so if the process creation library supported templating of the char type, then there wouldn't need to be conversion of wide char strings to narrow char strings. Plus, wouldn't there be a problem trying to execute a module on Windows that has a non-Latin character in it? Ok, I'll look into it! I don't remember anymore all the details why I gave up quickly in previous versions. As the implementation has been simplified maybe it's easier now to support wide chars somehow.
I think we should wait for encoding-aware string class. It will allow to solve most of i18n issues in many libraries in a uniform way. The libraries I mean are Boost.Asio, Boost.Interprocess, Boost.System, Boost.Process and so on.
This would be definitely my preferred solution! Such a library would help all others. Boris
On Fri, 20 Aug 2010 00:13:57 +0400, Ilya Sokolov wrote:
Boris Schaeling wrote: ... snip ... I think we should wait for encoding-aware string class.
You say 'wait' as though someone were actually working on this. Are they? Alex -- Swish - Easy SFTP for Windows Explorer (http://www.swish-sftp.org)
Alexander Lamaison wrote:
On Fri, 20 Aug 2010 00:13:57 +0400, Ilya Sokolov wrote:
Boris Schaeling wrote: ... snip ... I think we should wait for encoding-aware string class.
You say 'wait' as though someone were actually working on this. Are they?
- u16string and u32string from cpp0x - threating all char strings as UTF-8 Btw, even boost::filesystem3::path is an encoding-aware string class in some sense ).
On Fri, 20 Aug 2010 02:05:13 +0400, Ilya Sokolov wrote:
Alexander Lamaison wrote:
On Fri, 20 Aug 2010 00:13:57 +0400, Ilya Sokolov wrote:
Boris Schaeling wrote: ... snip ... I think we should wait for encoding-aware string class.
You say 'wait' as though someone were actually working on this. Are they?
- u16string and u32string from cpp0x
Ok. By encoding aware you mean fixed to a static encoding.
- threating all char strings as UTF-8
Difficult You can't do it blindly. char strings can be anything. On Windows they would _rarely_ be UTF-8.
Btw, even boost::filesystem3::path is an encoding-aware string class in some sense ).
That depends what we mean by encoding 'aware'. On Windows it assumes char strings are in the local codepage and wchar_t strings are UCS2. These are the same assumptions made by the underlying Windows API. Alex -- Swish - Easy SFTP for Windows Explorer (http://www.swish-sftp.org)
Alexander Lamaison wrote: > On Fri, 20 Aug 2010 02:05:13 +0400, Ilya Sokolov wrote: > >> Alexander Lamaison wrote: >>> On Fri, 20 Aug 2010 00:13:57 +0400, Ilya Sokolov wrote: >>> >>>> Boris Schaeling wrote: >>>> ... snip ... >>>> I think we should wait for encoding-aware string class. >>> You say 'wait' as though someone were actually working on this. Are they? >> - u16string and u32string from cpp0x > > Ok. By encoding aware you mean fixed to a static encoding. No
Daniel Trebbien wrote:
Is there a way to further remove platform dependence in handling the "exit code"? You mean the WIFEXITED et al macros? We had a status class in previous Boost.Process versions which encapsulated the exit code. However the design of the class was not really cross-platform as member functions like exited() were clearly added for POSIX systems only and didn't make sense on Windows (exited() for example always returned true). Thus the question is what to make platform-independent if a concept doesn't exist on a platform. :-)
The existence of boost::status would also force to know more than one interface to the same values.
A while ago when I was thinking about this same problem, I considered using Windows' debugging mechanism (process creation flag DEBUG_ONLY_THIS_PROCESS) to obtain this information. I don't know if debugging a process will affect its execution in some way, or whether the performance will be substantially impacted, but it might be interesting to see if this approach would work.
IIRC, DEBUG_ONLY_THIS_PROCESS requires special privilege.
Boris Schaeling wrote:
Here are the results of the Boost.Process project within this year's Google Summer of Code program: We have a complete library to manage system processes now!
If you like to play around with the library download the ZIP file from <http://www.highscore.de/boost/gsoc2010/process.zip>. As it's a header-only library simply copy the files to your Boost folder. If you like to quickly check the documentation click on <http://www.highscore.de/boost/gsoc2010/>.
Consider this program (untested): // simulate daemon process for (int i = 0; i < F_MAXFD; ++i) close(i); namespace bp == boost::process; bp::context ctx; // 1 ctx.stderr_behavior = bp::behavior::pipe::create( bp::behavior::pipe::output_stream); // 2 ctx.stdout_behaviour = bp::behavior::pipe::create( bp::behavior::pipe::output_stream); // 3 bp::child foo = bp::create_child("foo", std::vector<std::string>(), ctx); After line 1 we have: fd refers to 0 rend of pipe created for stderr 1 wend of pipe created for stderr After line 2: 0 rend of pipe created for stderr 1 wend of pipe created for stderr 2 rend of pipe created for stdout 3 wend of pipe created for stdout Now after line 3 the child process has the following fds: 0 closed 1 wend of pipe created for stdout 2 wend of pipe created for stdout (!) 3 closed
On Wed, 18 Aug 2010 11:23:56 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]Now after line 3 the child process has the following fds: 0 closed 1 wend of pipe created for stdout 2 wend of pipe created for stdout (!) 3 closed
Not sure why you think so? I've attached the program I used to try to reproduce the behavior but it works fine. Maybe you can update it if you think there is a problem? Boris
Boris Schaeling wrote:
On Wed, 18 Aug 2010 11:23:56 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]Now after line 3 the child process has the following fds: 0 closed 1 wend of pipe created for stdout 2 wend of pipe created for stdout (!) 3 closed
Not sure why you think so? I've attached the program I used to try to reproduce the behavior but it works fine. Maybe you can update it if you think there is a problem?
Could you swap these lines? ctx.stdout_behavior = bpb::pipe::create(bpb::pipe::output_stream); ctx.stderr_behavior = bpb::pipe::create(bpb::pipe::output_stream);
On Thu, 19 Aug 2010 21:17:08 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
Boris Schaeling wrote:
On Wed, 18 Aug 2010 11:23:56 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]Now after line 3 the child process has the following fds: 0 closed 1 wend of pipe created for stdout 2 wend of pipe created for stdout (!) 3 closed Not sure why you think so? I've attached the program I used to try to reproduce the behavior but it works fine. Maybe you can update it if you think there is a problem?
Could you swap these lines?
ctx.stdout_behavior = bpb::pipe::create(bpb::pipe::output_stream); ctx.stderr_behavior = bpb::pipe::create(bpb::pipe::output_stream);
I see, very good catch! File descriptor 3 (the write end of the pipe created for stdout) is mapped to STDOUT_FILENO with dup2(). As STDOUT_FILENO is file descriptor 1 that one is automatically closed by dup2(). In this use case file descriptor 1 is unfortunately the write end of the pipe created for stderr though. Any idea how to easily fix this? As far as I see the order of mapping read/write ends to stdin, stdout and stderr depends on whether another behavior actually uses the file descriptors 0, 1 and 2. In the worst case however the write end of the pipe created for stdout is 2 and the write end of the pipe created for stderr 1. Either we put a warning in the documentation or we come up with some clever code? (In the moment I'm a bit in a hurry. If there are no proposals I'll think about it on the weekend. :) By the way, I wonder if Boost.Test likes a test case which closes all file descriptors? It would be nice if this could be added as a test case. Boris
Boris Schaeling wrote:
On Thu, 19 Aug 2010 21:17:08 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
Boris Schaeling wrote:
On Wed, 18 Aug 2010 11:23:56 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]Now after line 3 the child process has the following fds: 0 closed 1 wend of pipe created for stdout 2 wend of pipe created for stdout (!) 3 closed Not sure why you think so? I've attached the program I used to try to reproduce the behavior but it works fine. Maybe you can update it if you think there is a problem?
Could you swap these lines?
ctx.stdout_behavior = bpb::pipe::create(bpb::pipe::output_stream); ctx.stderr_behavior = bpb::pipe::create(bpb::pipe::output_stream);
I see, very good catch! File descriptor 3 (the write end of the pipe created for stdout) is mapped to STDOUT_FILENO with dup2(). As STDOUT_FILENO is file descriptor 1 that one is automatically closed by dup2(). In this use case file descriptor 1 is unfortunately the write end of the pipe created for stderr though.
In other words, wend of pipe created for stdout overwrites wend of pipe created for stderr.
Any idea how to easily fix this? As far as I see the order of mapping read/write ends to stdin, stdout and stderr depends on whether another behavior actually uses the file descriptors 0, 1 and 2. In the worst case however the write end of the pipe created for stdout is 2 and the write end of the pipe created for stderr 1. Either we put a warning in the documentation or we come up with some clever code? (In the moment I'm a bit in a hurry. If there are no proposals I'll think about it on the weekend. :)
Three choices that I know of: 1. Before creating any new file (such as in bpb::pipe::create() or bpb::dummy::create(), not sure about bpb::inherit::create()), you could "occupy" fds which behavior has been set to != bpb::inherit. 2. Create new files in the ascending order of the corresponding fds. 3. Put this and other problems on the user, like in posix_spawn() http://www.opengroup.org/onlinepubs/009695399/functions/posix_spawn.html All choices are not thread-safe.
By the way, I wonder if Boost.Test likes a test case which closes all file descriptors? It would be nice if this could be added as a test case.
You could dup std descriptors to some upper fds before the test and dup them back after the test.
On Thu, 19 Aug 2010 23:58:08 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]
Any idea how to easily fix this? As far as I see the order of mapping read/write ends to stdin, stdout and stderr depends on whether another behavior actually uses the file descriptors 0, 1 and 2. In the worst case however the write end of the pipe created for stdout is 2 and the write end of the pipe created for stderr 1. Either we put a warning in the documentation or we come up with some clever code? (In the moment I'm a bit in a hurry. If there are no proposals I'll think about it on the weekend. :)
Three choices that I know of:
1. Before creating any new file (such as in bpb::pipe::create() or bpb::dummy::create(), not sure about bpb::inherit::create()), you could "occupy" fds which behavior has been set to != bpb::inherit.
Then a behavior class would somehow need to know what another behavior class has been doing? The constructors of the behavior classes would need access to the context object to check the other stream behaviors?
2. Create new files in the ascending order of the corresponding fds.
This would then be the responsibility of the library user (stdout_behavior would need to be set before stderr_behavior for example)?
3. Put this and other problems on the user, like in posix_spawn() http://www.opengroup.org/onlinepubs/009695399/functions/posix_spawn.html
Same as above (library user's responsibility)?
By the way, I wonder if Boost.Test likes a test case which closes all file descriptors? It would be nice if this could be added as a test case.
You could dup std descriptors to some upper fds before the test and dup them back after the test.
I will try this and hope Boost.Test doesn't use any other file descriptors. :-) Boris
On 21.08.2010 17:13, Boris Schaeling wrote:
On Thu, 19 Aug 2010 23:58:08 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
1. Before creating any new file (such as in bpb::pipe::create() or bpb::dummy::create(), not sure about bpb::inherit::create()), you could "occupy" fds which behavior has been set to != bpb::inherit.
Then a behavior class would somehow need to know what another behavior class has been doing? The constructors of the behavior classes would need access to the context object to check the other stream behaviors?
Not necessarily. create_child() could do all the job. In fact, it does in all other drafts.
2. Create new files in the ascending order of the corresponding fds.
This would then be the responsibility of the library user (stdout_behavior would need to be set before stderr_behavior for example)?
See above.
[snip]
By the way, I wonder if Boost.Test likes a test case which closes all file descriptors? It would be nice if this could be added as a test case.
You could dup std descriptors to some upper fds before the test and dup them back after the test.
I will try this and hope Boost.Test doesn't use any other file descriptors. :-)
)
On Sat, 21 Aug 2010 15:59:36 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 21.08.2010 17:13, Boris Schaeling wrote:
On Thu, 19 Aug 2010 23:58:08 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
1. Before creating any new file (such as in bpb::pipe::create() or bpb::dummy::create(), not sure about bpb::inherit::create()), you could "occupy" fds which behavior has been set to != bpb::inherit.
Then a behavior class would somehow need to know what another behavior class has been doing? The constructors of the behavior classes would need access to the context object to check the other stream behaviors?
Not necessarily. create_child() could do all the job. In fact, it does in all other drafts.
Problem fixed (in create_child() now) and test case checked in. I'll look into shell() next. Boris
Boris Schaeling wrote (in the attached file):
File descriptors must be closed after context is instantiated as the context constructor uses the behavior inherit which tries to dup() stdin, stdout and stderr.
Which makes me nervous (sorry) as it is superfluous in 90% of cases.
On Thu, 19 Aug 2010 22:34:36 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
Boris Schaeling wrote (in the attached file):
File descriptors must be closed after context is instantiated as the context constructor uses the behavior inherit which tries to dup() stdin, stdout and stderr.
Which makes me nervous (sorry) as it is superfluous in 90% of cases.
Currently the standard constructor of the context class reproduces standard behavior on POSIX systems (file descriptors are inherited). Do you think this should be changed? Or it would be fine to provide another constructor which expects up to three parameters to configure stdin, stdout and stderr? Boris
On Thu, 19 Aug 2010 22:40:14 +0200, Boris Schaeling <boris@highscore.de> wrote:
On Thu, 19 Aug 2010 22:34:36 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
Boris Schaeling wrote (in the attached file):
File descriptors must be closed after context is instantiated as the context constructor uses the behavior inherit which tries to dup() stdin, stdout and stderr.
Which makes me nervous (sorry) as it is superfluous in 90% of cases.
Currently the standard constructor of the context class reproduces standard behavior on POSIX systems (file descriptors are inherited). Do you think this should be changed? Or it would be fine to provide another constructor which expects up to three parameters to configure stdin, stdout and stderr?
Can you please comment on this one, too, Ilya? Maybe you missed this posting as the thread is rather huge now. Boris
On 25.08.2010 1:35, Boris Schaeling wrote:
On Thu, 19 Aug 2010 22:40:14 +0200, Boris Schaeling <boris@highscore.de> wrote:
On Thu, 19 Aug 2010 22:34:36 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
Boris Schaeling wrote (in the attached file):
File descriptors must be closed after context is instantiated as the context constructor uses the behavior inherit which tries to dup() stdin, stdout and stderr.
Which makes me nervous (sorry) as it is superfluous in 90% of cases.
Currently the standard constructor of the context class reproduces standard behavior on POSIX systems (file descriptors are inherited). Do you think this should be changed? Or it would be fine to provide another constructor which expects up to three parameters to configure stdin, stdout and stderr?
Can you please comment on this one, too, Ilya? Maybe you missed this posting as the thread is rather huge now.
I'll have some spare time this weekend. In the meantime, could you show the updated sources?
On Wed, 25 Aug 2010 00:02:24 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]I'll have some spare time this weekend. In the meantime, could you show the updated sources?
I've updated the ZIP file at <http://www.highscore.de/boost/gsoc2010/process.zip>. You also find the latest sources at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/>. Boris
On 25.08.2010 22:56, Boris Schaeling wrote:
On Wed, 25 Aug 2010 00:02:24 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]I'll have some spare time this weekend. In the meantime, could you show the updated sources?
I've updated the ZIP file at <http://www.highscore.de/boost/gsoc2010/process.zip>. You also find the latest sources at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/>.
Could somebody add this library with the links provided above to https://svn.boost.org/trac/boost/wiki/LibrariesUnderConstruction Thanks!
----- Original Message ----- From: "Ilya Sokolov" <ilyasokol@gmail.com> To: <boost@lists.boost.org> Sent: Tuesday, August 31, 2010 3:25 PM Subject: Re: [boost] [gsoc] Boost.Process done
On 25.08.2010 22:56, Boris Schaeling wrote:
On Wed, 25 Aug 2010 00:02:24 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
Could somebody add this library with the links provided above to https://svn.boost.org/trac/boost/wiki/LibrariesUnderConstruction
Done. I will add a link to the documentation. is there any available link? Best, Vicente
On 31.08.2010 20:55, vicente.botet wrote:
Could somebody add this library with the links provided above to https://svn.boost.org/trac/boost/wiki/LibrariesUnderConstruction
Done. I will add a link to the documentation. is there any available link?
http://www.highscore.de/boost/gsoc2010/ Thank you!
On 20.08.2010 0:40, Boris Schaeling wrote:
On Thu, 19 Aug 2010 22:34:36 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
Boris Schaeling wrote (in the attached file):
File descriptors must be closed after context is instantiated as the context constructor uses the behavior inherit which tries to dup() stdin, stdout and stderr.
Which makes me nervous (sorry) as it is superfluous in 90% of cases.
Currently the standard constructor of the context class reproduces standard behavior on POSIX systems (file descriptors are inherited). Do you think this should be changed?
No, but read below.
Or it would be fine to provide another constructor which expects up to three parameters to configure stdin, stdout and stderr?
I don't think it is good to create files in the constructors of classes derived from stream_behavior. Consider the following program (untested): bp::context ctx; ctx.stdout_behaviour = bp::behavior::pipe::create( bp::behavior::pipe::output_stream); // 1 bp::child foo = bp::create_child("foo", std::vector<std::string>(), ctx); // 2 bp::child bar = bp::create_child("bar", std::vector<std::string>(), ctx); AFAIU, the line 2 will throw exception "dup2() failed" on POSIX, and spawn child process with closed(!) stdout on Windows.
On Tue, 31 Aug 2010 15:47:38 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]I don't think it is good to create files in the constructors of classes derived from stream_behavior. Consider the following program (untested):
bp::context ctx; ctx.stdout_behaviour = bp::behavior::pipe::create( bp::behavior::pipe::output_stream);
// 1 bp::child foo = bp::create_child("foo", std::vector<std::string>(), ctx);
// 2 bp::child bar = bp::create_child("bar", std::vector<std::string>(), ctx);
AFAIU, the line 2 will throw exception "dup2() failed" on POSIX, and spawn child process with closed(!) stdout on Windows.
Yes, a context can not be reused. In the moment I have no idea though how to change context without making the interface more complicated. In some Boost.Process drafts stdout_behavior (and the other member variables) were enumerations: ctx.stdout_behavior = pipe; That was really simple but is neither extensible nor can't data be attached to a stream behavior (eg. a file descriptor to redirect to). I tried to make the new stream behavior classes a bit easier to use by adding the create() method as otherwise everyone would need to use boost::shared_ptr directly (and probably wonder what boost::shared_ptr has to do with setting stream behaviors). But I'm open for proposals how to improve context. Boris
On 01.09.2010 1:47, Boris Schaeling wrote:
On Tue, 31 Aug 2010 15:47:38 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]I don't think it is good to create files in the constructors of classes derived from stream_behavior. Consider the following program (untested):
bp::context ctx; ctx.stdout_behaviour = bp::behavior::pipe::create( bp::behavior::pipe::output_stream);
// 1 bp::child foo = bp::create_child("foo", std::vector<std::string>(), ctx);
// 2 bp::child bar = bp::create_child("bar", std::vector<std::string>(), ctx);
AFAIU, the line 2 will throw exception "dup2() failed" on POSIX, and spawn child process with closed(!) stdout on Windows.
Yes, a context can not be reused. In the moment I have no idea though how to change context without making the interface more complicated. In some Boost.Process drafts stdout_behavior (and the other member variables) were enumerations:
ctx.stdout_behavior = pipe;
That was really simple but is neither extensible nor can't data be attached to a stream behavior (eg. a file descriptor to redirect to).
I tried to make the new stream behavior classes a bit easier to use by adding the create() method as otherwise everyone would need to use boost::shared_ptr directly (and probably wonder what boost::shared_ptr has to do with setting stream behaviors). But I'm open for proposals how to improve context.
class stream // I don't like this name { public: virtual ~stream() {}; // create_child() calls this method virtual void create_parent_and_child_end(handle&, handle&); };
On 01.09.2010 18:55, Ilya Sokolov wrote:
class stream // I don't like this name { public: virtual ~stream() {}; // create_child() calls this method virtual void create_parent_and_child_end(handle&, handle&); };
Another try: class stream { public: virtual ~stream() {}; virtual void set_fileno(int); virtual std::pair<handle, handle> create_parent_and_child_end(); }; class inherit : public stream { public: void set_fileno(int); void create_parent_and_child_end(handle& pe, handle& ce) { handle pe; handle ce(GetStdHandle(DWORD(fileno_ - 10))); return std::make_pair(pe, ce); }; private: int fileno_; }; class context { public: context& stdin(stream s) { s.set_fileno(0); stdin_ = s; return *this; } ... };
On Wed, 01 Sep 2010 17:27:11 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 01.09.2010 18:55, Ilya Sokolov wrote:
class stream // I don't like this name { public: virtual ~stream() {}; // create_child() calls this method virtual void create_parent_and_child_end(handle&, handle&); };
Another try:
class stream { public: virtual ~stream() {}; virtual void set_fileno(int); virtual std::pair<handle, handle> create_parent_and_child_end(); };
class inherit : public stream { public: void set_fileno(int); void create_parent_and_child_end(handle& pe, handle& ce) { handle pe; handle ce(GetStdHandle(DWORD(fileno_ - 10))); return std::make_pair(pe, ce); };
private: int fileno_; };
class context { public: context& stdin(stream s) { s.set_fileno(0); stdin_ = s; return *this; } ... };
Looking at your proposed stream class I'd call it now a factory class. This makes sense to me and is even easier to explain (as many developers should be familiar with that pattern). The factory method could be called in create_child() making it possible to reuse contexts. But why did you add set_fileno()? Do we need this method if we have create_parent_and_child_end() already? The example looks like as if it's only required by inherit and thus shouldn't be added to stream really? Boris
On 01.09.2010 23:20, Boris Schaeling wrote:
[snip]
Looking at your proposed stream class I'd call it now a factory class. This makes sense to me and is even easier to explain (as many developers should be familiar with that pattern). The factory method could be called in create_child() making it possible to reuse contexts.
Yep.
But why did you add set_fileno()? Do we need this method if we have create_parent_and_child_end() already? The example looks like as if it's only required by inherit and thus shouldn't be added to stream really?
Probably. There is one more problem that worries me: ctx.stdin_behavior = bp::behavior::pipe::create(bp::behavior::pipe::input_stream); ^^^^^^^^^^^^ It is too easy to write output_stream here, and in 99% of cases its value is input_stream.
On Thu, 02 Sep 2010 00:21:36 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 01.09.2010 23:20, Boris Schaeling wrote:
[snip]
Looking at your proposed stream class I'd call it now a factory class. This makes sense to me and is even easier to explain (as many developers should be familiar with that pattern). The factory method could be called in create_child() making it possible to reuse contexts.
Yep.
But why did you add set_fileno()? Do we need this method if we have create_parent_and_child_end() already? The example looks like as if it's only required by inherit and thus shouldn't be added to stream really?
Probably. There is one more problem that worries me:
ctx.stdin_behavior = bp::behavior::pipe::create(bp::behavior::pipe::input_stream); ^^^^^^^^^^^^ It is too easy to write output_stream here, and in 99% of cases its value is input_stream.
At some point I thought of replacing get_child_end() and get_parent_end() with get_read_end() and get_write_end(). While stdin/stdout/stderr could then pick the right methods for a pipe automatically what would be the read and write end of the stream behaviors inherit or close? That's why I left it as it is. Any other ideas? Boris
On Wed, 01 Sep 2010 17:27:11 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 01.09.2010 18:55, Ilya Sokolov wrote:
class stream // I don't like this name { public: virtual ~stream() {}; // create_child() calls this method virtual void create_parent_and_child_end(handle&, handle&); };
Another try:
class stream { public: virtual ~stream() {}; virtual void set_fileno(int); virtual std::pair<handle, handle> create_parent_and_child_end(); };
class inherit : public stream { public: void set_fileno(int); void create_parent_and_child_end(handle& pe, handle& ce) { handle pe; handle ce(GetStdHandle(DWORD(fileno_ - 10))); return std::make_pair(pe, ce); };
private: int fileno_; };
class context { public: context& stdin(stream s) { s.set_fileno(0); stdin_ = s; return *this; } ... };
Here's another idea: How about using boost::function<> in the context class? Then stream behaviors don't need to be classes anymore, don't need to be in a class hierarchy and don't need to be created on the heap and stored in a boost::shared_ptr. struct context { boost::function<std::pair<handle, handle> ()> stdin_behavior; ... }; context ctx; ctx.stdin_behavior = &behavior::pipe; How does it look like? Boris
boost::process::shell must do signal handling. See [1] and [2]. On windows SetConsoleCtrlHandler() could be used for the same effect. [1] 10.18 system Function W. Richard Stevens, Stephen A. Rago. Advanced Programming in the UNIX® Environment, Second Edition http://www.apuebook.com/ [2] Specification of the system() function. http://www.opengroup.org/onlinepubs/000095399/functions/system.html
On Thu, 19 Aug 2010 23:05:15 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
boost::process::shell must do signal handling. See [1] and [2]. On windows SetConsoleCtrlHandler() could be used for the same effect.
[1] 10.18 system Function W. Richard Stevens, Stephen A. Rago. Advanced Programming in the UNIX® Environment, Second Edition http://www.apuebook.com/
[2] Specification of the system() function. http://www.opengroup.org/onlinepubs/000095399/functions/system.html
I read the specification but I'm not sure why it applies to boost::proces::shell(). For example boost::process::shell() does not wait for the child process to exit - why should SIGINT then be ignored? After all boost::process::shell() should not be a replacement for system()? Boris
On 22.08.2010 2:13, Boris Schaeling wrote:
On Thu, 19 Aug 2010 23:05:15 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
boost::process::shell must do signal handling. See [1] and [2]. On windows SetConsoleCtrlHandler() could be used for the same effect.
[1] 10.18 system Function W. Richard Stevens, Stephen A. Rago. Advanced Programming in the UNIX® Environment, Second Edition http://www.apuebook.com/
[2] Specification of the system() function. http://www.opengroup.org/onlinepubs/000095399/functions/system.html
I read the specification but I'm not sure why it applies to boost::proces::shell(). For example boost::process::shell() does not wait for the child process to exit - why should SIGINT then be ignored?
Ignore SIGINT until wait() is called.
After all boost::process::shell() should not be a replacement for system()?
Then what is the purpose of boost::process::shell()?
On 23.08.2010 15:34, Ilya Sokolov wrote:
On 22.08.2010 2:13, Boris Schaeling wrote:
On Thu, 19 Aug 2010 23:05:15 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
boost::process::shell must do signal handling. See [1] and [2]. On windows SetConsoleCtrlHandler() could be used for the same effect.
[1] 10.18 system Function W. Richard Stevens, Stephen A. Rago. Advanced Programming in the UNIX® Environment, Second Edition http://www.apuebook.com/
[2] Specification of the system() function. http://www.opengroup.org/onlinepubs/000095399/functions/system.html
I read the specification but I'm not sure why it applies to boost::proces::shell(). For example boost::process::shell() does not wait for the child process to exit - why should SIGINT then be ignored?
Ignore SIGINT until wait() is called.
After all boost::process::shell() should not be a replacement for system()?
Then what is the purpose of boost::process::shell()?
Forgive me if I am looking to aggressive (. It is a fair question.
On Mon, 23 Aug 2010 13:34:05 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 22.08.2010 2:13, Boris Schaeling wrote: [...]
I read the specification but I'm not sure why it applies to boost::proces::shell(). For example boost::process::shell() does not wait for the child process to exit - why should SIGINT then be ignored?
Ignore SIGINT until wait() is called.
After all boost::process::shell() should not be a replacement for system()?
Then what is the purpose of boost::process::shell()?
Let's say you want the list of files in the current work directory. While system("dir") will execute the command it is obviously useless. With boost::process::shell("dir") you can read the list of files through a stream though. In a way boost::process::shell() is nothing more than a convenient shortcut for boost::process::create_child("cmd.exe"...). Boris
On 15.08.2010 16:03, Boris Schaeling wrote:
* For the first time there is full support for synchronous and asynchronous operations.
Do you really need to call ConnectNamedPipe() at stream_behavior.hpp, line 258? AFAICS, at that time the client's end of pipe is opened already, so there is no need to wait for it.
On Mon, 23 Aug 2010 13:44:31 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 15.08.2010 16:03, Boris Schaeling wrote:
* For the first time there is full support for synchronous and asynchronous operations.
Do you really need to call ConnectNamedPipe() at stream_behavior.hpp, line 258? AFAICS, at that time the client's end of pipe is opened already, so there is no need to wait for it.
If I remember correctly ConnectNamedPipe() is called to make sure that both ends are connected before the constructor of boost::process::behavior::named_pipe returns. As CreateFileA() is called with FILE_FLAG_OVERLAPPED this function could return immediately and the program could continue without the parent end already open. Boris
On 23.08.2010 23:48, Boris Schaeling wrote:
On Mon, 23 Aug 2010 13:44:31 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 15.08.2010 16:03, Boris Schaeling wrote:
* For the first time there is full support for synchronous and asynchronous operations.
Do you really need to call ConnectNamedPipe() at stream_behavior.hpp, line 258? AFAICS, at that time the client's end of pipe is opened already, so there is no need to wait for it.
If I remember correctly ConnectNamedPipe() is called to make sure that both ends are connected before the constructor of boost::process::behavior::named_pipe returns. As CreateFileA() is called with FILE_FLAG_OVERLAPPED this function could return immediately
It couldn't because it returns "... an open handle to the specified file, device, named pipe, or mail slot ..."
and the program could continue without the parent end already open.
Boris
On Tue, 24 Aug 2010 08:43:46 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 23.08.2010 23:48, Boris Schaeling wrote: [...]
Do you really need to call ConnectNamedPipe() at stream_behavior.hpp, line 258? AFAICS, at that time the client's end of pipe is opened already, so there is no need to wait for it.
If I remember correctly ConnectNamedPipe() is called to make sure that both ends are connected before the constructor of boost::process::behavior::named_pipe returns. As CreateFileA() is called with FILE_FLAG_OVERLAPPED this function could return immediately
It couldn't because it returns "... an open handle to the specified file, device, named pipe, or mail slot ..."
I commented out the code and ran the relevant test cases a couple of times on Windows - they were all passed successfully. I don't know whether I misinterpreted anything on MSDN and thought that because of passing FILE_FLAG_OVERLAPPED to CreateFile() this function could also return immediately. I guess it's then safe to remove the call to ConnectNamedPipe() (if not we simply revert to the old version in Subversion :-). Boris
On Sun, Aug 15, 2010 at 8:03 AM, Boris Schaeling <boris@highscore.de> wrote:
If you like to play around with the library download the ZIP file from <http://www.highscore.de/boost/gsoc2010/process.zip>. As it's a header-only library simply copy the files to your Boost folder. If you like to quickly
What's the minimum version of Boost it will work with? The latest version my employer has approved is 1.39 and I can see it doesn't work with that because UUID doesn't even exist.
On Wed, 25 Aug 2010 00:27:35 +0200, John B. Turpish <jbturp@gmail.com> wrote:
[...]What's the minimum version of Boost it will work with? The latest version my employer has approved is 1.39 and I can see it doesn't work with that because UUID doesn't even exist.
I tested everything with 1.43. In general Boost.Process depends on these header files from other Boost C++ libraries: boost/algorithm/string/predicate.hpp boost/asio.hpp boost/assert.hpp boost/bind.hpp boost/config.hpp boost/filesystem/path.hpp boost/lexical_cast.hpp boost/make_shared.hpp boost/noncopyable.hpp boost/preprocessor/stringize.hpp boost/ptr_container/ptr_unordered_map.hpp boost/scoped_array.hpp boost/shared_array.hpp boost/shared_ptr.hpp boost/system/config.hpp boost/system/system_error.hpp boost/thread.hpp boost/throw_exception.hpp boost/uuid/random_generator.hpp boost/uuid/uuid.hpp boost/uuid/uuid_io.hpp If you don't include boost/process.hpp or boost/process/all.hpp though but only those header files you need then boost/asio.hpp and a couple of others are not required (if you don't do any asynchronous I/O for example). But you are right that Boost.UUID is always required. The reason is that a unique named pipe must be created if behavior::named_pipe is used. If there are other ideas how to generate a unique name I can update the code (Windows provides an API to generate UUIDs but unfortunately it requires COM; on POSIX systems I'm not aware of any system function which could generate UUIDs). Boris
On 25.08.2010 23:36, Boris Schaeling wrote:
In general Boost.Process depends on these header files from other Boost C++ libraries:
boost/algorithm/string/predicate.hpp
I don't think its use is justified.
boost/filesystem/path.hpp boost/lexical_cast.hpp
ditto
boost/preprocessor/stringize.hpp
BOOST_STRINGIZE (from Boost.Config) could be used instead of BOOST_PP_STRINGIZE
boost/thread.hpp
See my other reply about BOOST_PROCESS_NO_THREAD
boost/uuid/random_generator.hpp boost/uuid/uuid.hpp boost/uuid/uuid_io.hpp
I'll comment on these later
On Tue, 31 Aug 2010 14:48:43 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 25.08.2010 23:36, Boris Schaeling wrote:
In general Boost.Process depends on these header files from other Boost C++ libraries:
boost/algorithm/string/predicate.hpp
I don't think its use is justified.
Quickly checked what is used where: find_executable_in_path() uses boost::algorithm::ends_with() and boost::algorithm::iends_with().
boost/filesystem/path.hpp boost/lexical_cast.hpp
ditto
find_executable_in_path() throws boost::filesystem::filesystem_error() if a program isn't found in PATH. boost::lexical_cast is used by named_pipe to convert the UUID (from Boost.Uuid) to a std::string.
boost/preprocessor/stringize.hpp
BOOST_STRINGIZE (from Boost.Config) could be used instead of BOOST_PP_STRINGIZE
Changed. Regarding the header files above: I wonder whether it makes sense to get rid of actual helpful functions like boost::algorithm::ends_with() only to remove a dependency on another Boost library. I understand the case about Boost.Filesystem which requires to be built. But it's a kind of strange goal for Boost libraries to reuse as little code as possible from other Boost libraries? Boris
[...]
Boris Schaeling wrote:
On Tue, 31 Aug 2010 14:48:43 +0200, Ilya Sokolov wrote:
In general Boost.Process depends on these header files from other Boost C++ libraries:
boost/algorithm/string/predicate.hpp
I don't think its use is justified.
You should clearly state your grounds for the objection.
Quickly checked what is used where: find_executable_in_path() uses boost::algorithm::ends_with() and boost::algorithm::iends_with().
boost/filesystem/path.hpp boost/lexical_cast.hpp
ditto
find_executable_in_path() throws boost::filesystem::filesystem_error() if a program isn't found in PATH. boost::lexical_cast is used by named_pipe to convert the UUID (from Boost.Uuid) to a std::string.
That code from the included headers is used is, indeed, justification for their inclusion. Any argument against that functionality should include an alternative implementation of the required functionality and a justification for choosing the alternatives.
Regarding the header files above: I wonder whether it makes sense to get rid of actual helpful functions like boost::algorithm::ends_with() only to remove a dependency on another Boost library. I understand the case about Boost.Filesystem which requires to be built. But it's a kind of strange goal for Boost libraries to reuse as little code as possible from other Boost libraries?
I agree. This has reared its head before. While gratuitous and superfluous dependencies should be avoided, the whole point of Boost is to provide ready made, well tested libraries of reusable code. If one Boost library cannot use another, why would we expect anyone else to use Boost libraries? A dependency on Boost.Filesystem is reasonable to avoid, if possible, because it means Boost.Process could no longer be considered header only. However, filesystem_error seems a perfectly logical choice and, I presume, it would be expected by anyone already using Boost.Filesystem. More significantly, there was some mention of using boost::filesystem::path, IIRC, so the dependency exists independent of that error type. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Stewart, Robert wrote:
Boris Schaeling wrote:
On Tue, 31 Aug 2010 14:48:43 +0200, Ilya Sokolov wrote:
In general Boost.Process depends on these header files from other Boost C++ libraries:
boost/algorithm/string/predicate.hpp I don't think its use is justified.
You should clearly state your grounds for the objection.
Quickly checked what is used where: find_executable_in_path() uses boost::algorithm::ends_with() and boost::algorithm::iends_with().
boost/filesystem/path.hpp boost/lexical_cast.hpp ditto find_executable_in_path() throws boost::filesystem::filesystem_error() if a program isn't found in PATH. boost::lexical_cast is used by named_pipe to convert the UUID (from Boost.Uuid) to a std::string.
That code from the included headers is used is, indeed, justification for their inclusion. Any argument against that functionality should include an alternative implementation of the required functionality and a justification for choosing the alternatives.
Regarding the header files above: I wonder whether it makes sense to get rid of actual helpful functions like boost::algorithm::ends_with() only to remove a dependency on another Boost library. I understand the case about Boost.Filesystem which requires to be built. But it's a kind of strange goal for Boost libraries to reuse as little code as possible from other Boost libraries?
I agree. This has reared its head before. While gratuitous and superfluous dependencies should be avoided, the whole point of Boost is to provide ready made, well tested libraries of reusable code. If one Boost library cannot use another, why would we expect anyone else to use Boost libraries?
A dependency on Boost.Filesystem is reasonable to avoid, if possible, because it means Boost.Process could no longer be considered header only. However, filesystem_error seems a perfectly logical choice and, I presume, it would be expected by anyone already using Boost.Filesystem. More significantly, there was some mention of using boost::filesystem::path, IIRC, so the dependency exists independent of that error type.
I would prefer that filesystem::path be used rather than strings that represent filenames. This would negate the usage of boost.algorithm since it's usage is replicating fundamental filesystem functionality. Most of the code in 'find_executable_in_path' could be replaced with a few lines of portable filesystem code. At a minimum, 'find_executable_in_path' should be pulled out of operations.hpp and placed in it's own header/cpp. Alternatively filesystem could provide this function. Searching for an executable differs little from searching the environment path for a dll, or searching a list of directories for includes/libs. Jeff
On 01.09.2010 14:51, Stewart, Robert wrote:
Boris Schaeling wrote:
On Tue, 31 Aug 2010 14:48:43 +0200, Ilya Sokolov wrote:
In general Boost.Process depends on these header files from other Boost C++ libraries:
boost/algorithm/string/predicate.hpp
I don't think its use is justified.
You should clearly state your grounds for the objection.
Ok, see my other reply.
Quickly checked what is used where: find_executable_in_path() uses boost::algorithm::ends_with() and boost::algorithm::iends_with().
boost/filesystem/path.hpp boost/lexical_cast.hpp
ditto
find_executable_in_path() throws boost::filesystem::filesystem_error() if a program isn't found in PATH. boost::lexical_cast is used by named_pipe to convert the UUID (from Boost.Uuid) to a std::string.
That code from the included headers is used is, indeed, justification for their inclusion.
Any argument against that functionality should include an alternative implementation of the required functionality
Not sure
and a justification for choosing the alternatives.
Agreed.
Regarding the header files above: I wonder whether it makes sense to get rid of actual helpful functions like boost::algorithm::ends_with() only to remove a dependency on another Boost library. I understand the case about Boost.Filesystem which requires to be built. But it's a kind of strange goal for Boost libraries to reuse as little code as possible from other Boost libraries?
I agree. This has reared its head before. While gratuitous and superfluous dependencies should be avoided, the whole point of Boost is to provide ready made, well tested libraries of reusable code. If one Boost library cannot use another, why would we expect anyone else to use Boost libraries?
Why would we expect anyone to use some Boost library, if its usage is impossible without all other Boost libraries?
A dependency on Boost.Filesystem is reasonable to avoid, if possible, because it means Boost.Process could no longer be considered header only. However, filesystem_error seems a perfectly logical choice and, I presume, it would be expected by anyone already using Boost.Filesystem. More significantly, there was some mention of using boost::filesystem::path, IIRC, so the dependency exists independent of that error type.
"some mention of using boost::filesystem::path" doesn't mean that "the dependency exists independent of that error type".
Ilya Sokolov wrote:
On 01.09.2010 14:51, Stewart, Robert wrote:
there was some mention of using boost::filesystem::path, IIRC, so the dependency exists independent of that error type.
"some mention of using boost::filesystem::path" doesn't mean that "the dependency exists independent of that error type".
Certainly. My using "some mention" was because I thought I'd read something about path being used already, not that it was a suggestion mentioned in passing. That's why I used "IIRC." I was implying that if, indeed, path is used elsewhere -- and I don't know that -- then there is already a dependency. I was also allowing for the possibility that if path will appear soon in an update, if indeed it isn't used presently, the dependency would arise at that time and justify using the error type now. Only in the case that path is not used now and isn't used in an upcoming update does the error type remain a problem. It is good to avoid new exception types when reasonable. If there is no other reason to depend upon Boost.Filesystem -- Jeff Flynn has suggested there is -- another exception type is certainly appropriate. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 01.09.2010 2:41, Boris Schaeling wrote:
On Tue, 31 Aug 2010 14:48:43 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 25.08.2010 23:36, Boris Schaeling wrote:
In general Boost.Process depends on these header files from other Boost C++ libraries:
boost/algorithm/string/predicate.hpp
I don't think its use is justified.
Quickly checked what is used where: find_executable_in_path() uses boost::algorithm::ends_with() and boost::algorithm::iends_with().
Both usages could be replaced with two or three lines, while boost/algorithm/string/predicate.hpp depends on Boost.Range, which depends on Boost.TypeTraits, Boost.MPL and so on.
boost/filesystem/path.hpp boost/lexical_cast.hpp
ditto
find_executable_in_path() throws boost::filesystem::filesystem_error() if a program isn't found in PATH.
IMO, it doesn't matter what find_executable_in_path() throws, it could be any other class without any pain for users.
boost::lexical_cast is used by named_pipe to convert the UUID (from Boost.Uuid) to a std::string.
boost/lexical_cast.hpp includes almost everything.
boost/preprocessor/stringize.hpp
BOOST_STRINGIZE (from Boost.Config) could be used instead of BOOST_PP_STRINGIZE
Changed.
Regarding the header files above: I wonder whether it makes sense to get rid of actual helpful functions like boost::algorithm::ends_with() only to remove a dependency on another Boost library. I understand the case about Boost.Filesystem which requires to be built. But it's a kind of strange goal for Boost libraries to reuse as little code as possible from other Boost libraries?
"Negative effects of coupling become obvious when one library uses a second library which uses a third, and so on. The worst form of coupling requires the user understand each of the coupled libraries. Coupling may also reduce the portability of a library - even in case when all used libraries are self-sufficient..." http://www.boost.org/development/reuse.html
On Wed, 01 Sep 2010 15:47:27 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]
Regarding the header files above: I wonder whether it makes sense to get rid of actual helpful functions like boost::algorithm::ends_with() only to remove a dependency on another Boost library. I understand the case about Boost.Filesystem which requires to be built. But it's a kind of strange goal for Boost libraries to reuse as little code as possible from other Boost libraries?
"Negative effects of coupling become obvious when one library uses a second library which uses a third, and so on. The worst form of coupling requires the user understand each of the coupled libraries. Coupling may also reduce the portability of a library - even in case when all used libraries are self-sufficient..."
I quote then the paragraph starting with "The benefits of using components from other libraries...". ;) Anyway, while it all sounds good in theory how many developers are out there trying to use single Boost libraries without installing the others? It sounds like a bit too much effort reinventing what has been implemented in other Boost libraries for what I expect is a relatively small group of developers? I heard much more often that developers didn't want to build libraries but simply include header files. That said I wouldn't mind to drop the dependency on Boost.Filesystem as currenly it is indeed only used because of an exception class. As I think this is the right class to use though design decisions will then be affected by library dependencies! Are we sure that we want this? Boris
On 01.09.2010 23:54, Boris Schaeling wrote:
On Wed, 01 Sep 2010 15:47:27 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]
Regarding the header files above: I wonder whether it makes sense to get rid of actual helpful functions like boost::algorithm::ends_with() only to remove a dependency on another Boost library. I understand the case about Boost.Filesystem which requires to be built. But it's a kind of strange goal for Boost libraries to reuse as little code as possible from other Boost libraries?
"Negative effects of coupling become obvious when one library uses a second library which uses a third, and so on. The worst form of coupling requires the user understand each of the coupled libraries. Coupling may also reduce the portability of a library - even in case when all used libraries are self-sufficient..."
I quote then the paragraph starting with "The benefits of using components from other libraries...". ;)
Anyway, while it all sounds good in theory how many developers are out there trying to use single Boost libraries without installing the others?
Actually, I wasn't thinking about such developers, but about compilation time (lexical cast and algo.str), portability and stability (filesystem).
It sounds like a bit too much effort reinventing what has been implemented in other Boost libraries for what I expect is a relatively small group of developers?
The dependencies I object to are easy to avoid.
On Wed, 01 Sep 2010 23:51:07 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
I quote then the paragraph starting with "The benefits of using components from other libraries...". ;)
Anyway, while it all sounds good in theory how many developers are out there trying to use single Boost libraries without installing the others?
Actually, I wasn't thinking about such developers, but about compilation time (lexical cast and algo.str), portability and stability (filesystem).
It sounds like a bit too much effort reinventing what has been implemented in other Boost libraries for what I expect is a relatively small group of developers?
The dependencies I object to are easy to avoid.
Regarding boost::lexical_cast: There is to_string() in Boost.Uuid in 1.44 but not in 1.43. Or what were you thinking of? I'll check the other dependencies tomorrow (or patches if sent :). Boris
On 02.09.2010 2:09, Boris Schaeling wrote:
On Wed, 01 Sep 2010 23:51:07 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
I quote then the paragraph starting with "The benefits of using components from other libraries...". ;)
Anyway, while it all sounds good in theory how many developers are out there trying to use single Boost libraries without installing the others?
Actually, I wasn't thinking about such developers, but about compilation time (lexical cast and algo.str), portability and stability (filesystem).
It sounds like a bit too much effort reinventing what has been implemented in other Boost libraries for what I expect is a relatively small group of developers?
The dependencies I object to are easy to avoid.
Regarding boost::lexical_cast: There is to_string() in Boost.Uuid in 1.44 but not in 1.43. Or what were you thinking of?
I was thinking about sprintf or ostringstream. If you add async_pipe, there will be no need for Boost.Uuid
On Thu, 02 Sep 2010 00:36:33 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 02.09.2010 2:09, Boris Schaeling wrote:
On Wed, 01 Sep 2010 23:51:07 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
I quote then the paragraph starting with "The benefits of using components from other libraries...". ;)
Anyway, while it all sounds good in theory how many developers are out there trying to use single Boost libraries without installing the others?
Actually, I wasn't thinking about such developers, but about compilation time (lexical cast and algo.str), portability and stability (filesystem).
It sounds like a bit too much effort reinventing what has been implemented in other Boost libraries for what I expect is a relatively small group of developers?
The dependencies I object to are easy to avoid.
Regarding boost::lexical_cast: There is to_string() in Boost.Uuid in 1.44 but not in 1.43. Or what were you thinking of?
I was thinking about sprintf or ostringstream. If you add async_pipe, there will be no need for Boost.Uuid
I don't want to give up named_pipe entirely as we don't know if someone has a need for it. I see then three possibilities: 1) Macro to exclude named_pipe 2) Move named_pipe to another header file 3) Move all behavior types to a subfolder called behavior and create a header file for each type Any preference? Or is it possible to detect whether a library is available? Are there somewhere (eg. in Boost.Config) macros defined like BOOST_LIBRARY_UUID? If it's not defined Boost.Process would know it shouldn't try to include boost/uuid.hpp. Boris
On 03.09.2010 0:27, Boris Schaeling wrote:
On Thu, 02 Sep 2010 00:36:33 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 02.09.2010 2:09, Boris Schaeling wrote:
On Wed, 01 Sep 2010 23:51:07 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
I quote then the paragraph starting with "The benefits of using components from other libraries...". ;)
Anyway, while it all sounds good in theory how many developers are out there trying to use single Boost libraries without installing the others?
Actually, I wasn't thinking about such developers, but about compilation time (lexical cast and algo.str), portability and stability (filesystem).
It sounds like a bit too much effort reinventing what has been implemented in other Boost libraries for what I expect is a relatively small group of developers?
The dependencies I object to are easy to avoid.
Regarding boost::lexical_cast: There is to_string() in Boost.Uuid in 1.44 but not in 1.43. Or what were you thinking of?
I was thinking about sprintf or ostringstream. If you add async_pipe, there will be no need for Boost.Uuid
I don't want to give up named_pipe entirely as we don't know if someone has a need for it.
Sorry for my obscure explanations, I meant that we could have the following kinds of pipe: - pipe (i.e. anonymous pipe) - named_pipe (requires a name as an argument, no need to generate it) - async_pipe - POSIX: typedef'ed to pipe - Windows: derived from named_pipe, uses UuidCreateSequential() for the random name HTH
On Thu, 02 Sep 2010 22:58:27 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 03.09.2010 0:27, Boris Schaeling wrote:
On Thu, 02 Sep 2010 00:36:33 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 02.09.2010 2:09, Boris Schaeling wrote:
On Wed, 01 Sep 2010 23:51:07 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
I quote then the paragraph starting with "The benefits of using components from other libraries...". ;)
Anyway, while it all sounds good in theory how many developers are out there trying to use single Boost libraries without installing the others?
Actually, I wasn't thinking about such developers, but about compilation time (lexical cast and algo.str), portability and stability (filesystem).
It sounds like a bit too much effort reinventing what has been implemented in other Boost libraries for what I expect is a relatively small group of developers?
The dependencies I object to are easy to avoid.
Regarding boost::lexical_cast: There is to_string() in Boost.Uuid in 1.44 but not in 1.43. Or what were you thinking of?
I was thinking about sprintf or ostringstream. If you add async_pipe, there will be no need for Boost.Uuid
I don't want to give up named_pipe entirely as we don't know if someone has a need for it.
Sorry for my obscure explanations, I meant that we could have the following kinds of pipe:
- pipe (i.e. anonymous pipe) - named_pipe (requires a name as an argument, no need to generate it) - async_pipe - POSIX: typedef'ed to pipe - Windows: derived from named_pipe, uses UuidCreateSequential() for the random name
Ah, I see - makes sense. If you agree that boost::function<> makes even more sense (see my other posting today) I'll update Boost.Process accordingly. Boris
On 25.08.2010 23:36, Boris Schaeling wrote:
On Wed, 25 Aug 2010 00:27:35 +0200, John B. Turpish <jbturp@gmail.com> wrote:
[...]What's the minimum version of Boost it will work with? The latest version my employer has approved is 1.39 and I can see it doesn't work with that because UUID doesn't even exist.
I tested everything with 1.43.
In general Boost.Process depends on these header files from other Boost C++ libraries:
[snip] boost/bind.hpp
Seems superfluous in basic_status_service.hpp.
[snip]
On Wed, 01 Sep 2010 23:52:45 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 25.08.2010 23:36, Boris Schaeling wrote:
On Wed, 25 Aug 2010 00:27:35 +0200, John B. Turpish <jbturp@gmail.com> wrote:
[...]What's the minimum version of Boost it will work with? The latest version my employer has approved is 1.39 and I can see it doesn't work with that because UUID doesn't even exist.
I tested everything with 1.43.
In general Boost.Process depends on these header files from other Boost C++ libraries:
[snip] boost/bind.hpp
Seems superfluous in basic_status_service.hpp.
Removed. Boris
Hi Boris, Mac XCode gcc4.0.1 warns about missing virtual destructors on classes with virtual functions for: boost::process::behavior:: stream inherit pipe named_pipe dummy Thanks, Jeff
On Thu, 26 Aug 2010 22:41:46 +0200, Jeff Flinn <TriumphSprint2000@hotmail.com> wrote:
Hi Boris,
Mac XCode gcc4.0.1 warns about missing virtual destructors on classes with virtual functions for:
boost::process::behavior:: stream inherit pipe named_pipe dummy
I've added an empty virtual destructor to the base class behavior::stream. Thanks, Boris
Boris Schaeling wrote:
Here are the results of the Boost.Process project within this year's
Is this intended: boost/process/stream_behavior.hpp: line 117: return boost::make_shared<inherit>(inherit(child_end)); This is constructing an 'inherit' instance and passing it to make_shared, which I presume then calls the default copy constructor for inherit. Don't you mean: return boost::make_shared<inherit>(child_end); Thanks, Jeff
Jeff Flinn wrote:
Boris Schaeling wrote:
Here are the results of the Boost.Process project within this year's
Is this intended:
boost/process/stream_behavior.hpp: line 117:
return boost::make_shared<inherit>(inherit(child_end));
I see this is done in almost all uses of make_shared within boost::process.
This is constructing an 'inherit' instance and passing it to make_shared, which I presume then calls the default copy constructor for inherit. Don't you mean:
return boost::make_shared<inherit>(child_end);
Jeff
On Fri, 27 Aug 2010 04:44:17 +0200, Jeff Flinn <TriumphSprint2000@hotmail.com> wrote:
Jeff Flinn wrote:
Boris Schaeling wrote:
Here are the results of the Boost.Process project within this year's Is this intended: boost/process/stream_behavior.hpp: line 117: return boost::make_shared<inherit>(inherit(child_end));
I see this is done in almost all uses of make_shared within boost::process.
Thanks, fixed in Subversion and in the ZIP file at <http://www.highscore.de/boost/gsoc2010/process.zip>! Boris
Boris Schaeling wrote:
For now I would appreciate if the library's test cases are run on as many platforms as possible. After copying the files from the ZIP file to the Boost folder change to libs/process/test/ and run bjam - that's all!
Under FreeBSD 8 with gcc 4.5.0, the tests ran fine except 'wait', which failed to link due to undefined reference to pthread_create. Adding <target-os>freebsd:<linkflags>-pthread to test project's requirements fixed that for me. Best Regards, Gevorg
On Sun, 29 Aug 2010 22:59:42 +0200, Gevorg Voskanyan <v_gevorg@yahoo.com> wrote:
Boris Schaeling wrote:
For now I would appreciate if the library's test cases are run on as many platforms as possible. After copying the files from the ZIP file to the Boost folder change to libs/process/test/ and run bjam - that's all!
Under FreeBSD 8 with gcc 4.5.0, the tests ran fine except 'wait', which failed to link due to undefined reference to pthread_create. Adding <target-os>freebsd:<linkflags>-pthread to test project's requirements fixed that for me.
Thank you very much for running the test cases! Looking at the values <target-os> can be set to I wonder which other plaforms need -pthread: aix, bsd, cygwin, darwin, freebsd, hpux, iphone, linux, netbsd, openbsd, osf, qnx, qnxnto, sgi, solaris, unix, unixware, windows So far it's Linux and FreeBSD. Or does unix mean it's set for all Unix-like platforms? Boris
Boris Schaeling wrote:
Thank you very much for running the test cases! Looking at the values <target-os> can be set to I wonder which other plaforms need -pthread:
aix, bsd, cygwin, darwin, freebsd, hpux, iphone, linux, netbsd, openbsd, osf, qnx, qnxnto, sgi, solaris, unix, unixware, windows
So far it's Linux and FreeBSD. Or does unix mean it's set for all Unix-like platforms?
Apparently not - having <target-os>unix:<linkflags>-pthread gives the same 'undefined reference' error, so I guess it is for the commercial Unix OS. But then using <target-os>bsd doesn't work either, so I wonder what <target-os>bsd is for. Is there still any original BSD OS out there in use? I'm cross-posting this to Boost.Build mailing list hoping some Boost.Build guru could clarify this... Thanks, Gevorg
Boris Schaeling wrote:
On Sun, 29 Aug 2010 22:59:42 +0200, Gevorg Voskanyan <v_gevorg@yahoo.com> wrote:
Boris Schaeling wrote:
For now I would appreciate if the library's test cases are run on as many platforms as possible. After copying the files from the ZIP file to the Boost folder change to libs/process/test/ and run bjam - that's all!
Under FreeBSD 8 with gcc 4.5.0, the tests ran fine except 'wait', which failed to link due to undefined reference to pthread_create. Adding <target-os>freebsd:<linkflags>-pthread to test project's requirements fixed that for me.
Thank you very much for running the test cases! Looking at the values <target-os> can be set to I wonder which other plaforms need -pthread:
aix, bsd, cygwin, darwin, freebsd, hpux, iphone, linux, netbsd, openbsd, osf, qnx, qnxnto, sgi, solaris, unix, unixware, windows
So far it's Linux and FreeBSD. Or does unix mean it's set for all Unix-like platforms?
Well, if you're using pthread_create, then using threading=multi seems like a better approach than linking to the pthread library directly. Is this not good idea for some reason? - Volodya
On Mon, 30 Aug 2010 08:52:00 +0200, Vladimir Prus <vladimir@codesourcery.com> wrote:
[...]
Thank you very much for running the test cases! Looking at the values <target-os> can be set to I wonder which other plaforms need -pthread:
aix, bsd, cygwin, darwin, freebsd, hpux, iphone, linux, netbsd, openbsd, osf, qnx, qnxnto, sgi, solaris, unix, unixware, windows
So far it's Linux and FreeBSD. Or does unix mean it's set for all Unix-like platforms?
Well, if you're using pthread_create, then using threading=multi seems like a better approach than linking to the pthread library directly. Is this not good idea for some reason?
I checked now why I even had to add -lpthread if it wasn't required in earlier Boost.Process drafts: The new status class uses a worker thread to support asynchronous I/O. So yes, threading=multi is the natural choice. ;) Updated in SVN and in the ZIP file. Boris
Awesome work. Glad I'll be able to start tinkering with it again. On Aug 30, 2010, at 2:59 PM, Boris Schaeling wrote:
On Mon, 30 Aug 2010 08:52:00 +0200, Vladimir Prus <vladimir@codesourcery.com> wrote:
[...]
Thank you very much for running the test cases! Looking at the values <target-os> can be set to I wonder which other plaforms need -pthread:
aix, bsd, cygwin, darwin, freebsd, hpux, iphone, linux, netbsd, openbsd, osf, qnx, qnxnto, sgi, solaris, unix, unixware, windows
So far it's Linux and FreeBSD. Or does unix mean it's set for all Unix-like platforms?
Well, if you're using pthread_create, then using threading=multi seems like a better approach than linking to the pthread library directly. Is this not good idea for some reason?
I checked now why I even had to add -lpthread if it wasn't required in earlier Boost.Process drafts: The new status class uses a worker thread to support asynchronous I/O. So yes, threading=multi is the natural choice. ;) Updated in SVN and in the ZIP file.
Boris
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On 30.08.2010 22:59, Boris Schaeling wrote:
I checked now why I even had to add -lpthread if it wasn't required in earlier Boost.Process drafts: The new status class uses a worker thread to support asynchronous I/O. So yes, threading=multi is the natural choice. ;) Updated in SVN and in the ZIP file.
I expect bp::status::async_wait() to be used rarely enough, to some degree where BOOST_PROCESS_NO_THREAD is justified.
Hi, is this library already considered "in review"? I don't see it in the review queue and I guess it's because this last developpement was a SOC project. Joël Lamotte On Tue, Aug 31, 2010 at 13:47, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 30.08.2010 22:59, Boris Schaeling wrote:
I checked now why I even had to add -lpthread if it wasn't required in earlier Boost.Process drafts: The new status class uses a worker thread to support asynchronous I/O. So yes, threading=multi is the natural choice. ;) Updated in SVN and in the ZIP file.
I expect bp::status::async_wait() to be used rarely enough, to some degree where BOOST_PROCESS_NO_THREAD is justified.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Tue, 31 Aug 2010 14:03:59 +0200, Klaim <mjklaim@gmail.com> wrote:
is this library already considered "in review"? I don't see it in the review queue and I guess it's because this last developpement was a SOC project.
Officially this is not a review. Given that the very first version of Boost.Process was created in 2006 and that there have been several attempts to complete this library since then (among others by Ilya and myself in 2008) I would really appreciate if this library could have a chance (=review) to become an official Boost library soon. As Ilya knows the various Boost.Process drafts inside out I would also like him to be the review manager (I should ask him first though :). I've no idea if this complies with official procedures (as there is a review queue for example). But then I don't know if we ever get this done. ;) Boris
On 01.09.2010 2:04, Boris Schaeling wrote:
On Tue, 31 Aug 2010 14:03:59 +0200, Klaim <mjklaim@gmail.com> wrote:
is this library already considered "in review"? I don't see it in the review queue and I guess it's because this last developpement was a SOC project.
Officially this is not a review. Given that the very first version of Boost.Process was created in 2006 and that there have been several attempts to complete this library since then (among others by Ilya and myself in 2008) I would really appreciate if this library could have a chance (=review) to become an official Boost library soon. As Ilya knows the various Boost.Process drafts inside out I would also like him to be the review manager (I should ask him first though :).
Sorry, but I don't have much time and my own opinion is biased.
On Tue, 31 Aug 2010 13:47:53 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
On 30.08.2010 22:59, Boris Schaeling wrote:
I checked now why I even had to add -lpthread if it wasn't required in earlier Boost.Process drafts: The new status class uses a worker thread to support asynchronous I/O. So yes, threading=multi is the natural choice. ;) Updated in SVN and in the ZIP file.
I expect bp::status::async_wait() to be used rarely enough, to some degree where BOOST_PROCESS_NO_THREAD is justified.
I have no strong opinion. I wouldn't care but if it helps others I can add it? Boris
On 08/30/2010 11:59 AM, Boris Schaeling wrote:
[snip]
I checked now why I even had to add -lpthread if it wasn't required in earlier Boost.Process drafts: The new status class uses a worker thread to support asynchronous I/O. So yes, threading=multi is the natural choice. ;) Updated in SVN and in the ZIP file.
The use of a worker thread seems like an unfortunate additional burden. I don't know about the Windows implementation, but on Linux it seems in principle it should be possible to handle it via a SIGCHLD handler.
On Thu, 09 Sep 2010 18:25:57 -0700 Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
On 08/30/2010 11:59 AM, Boris Schaeling wrote:
[snip]
I checked now why I even had to add -lpthread if it wasn't required in earlier Boost.Process drafts: The new status class uses a worker thread to support asynchronous I/O. So yes, threading=multi is the natural choice. ;) Updated in SVN and in the ZIP file.
The use of a worker thread seems like an unfortunate additional burden. I don't know about the Windows implementation, but on Linux it seems in principle it should be possible to handle it via a SIGCHLD handler.
Callback would need to be async-signal-safe. If a user worries about performance so much, she could set up signal handler itself.
On 09/10/2010 02:25 PM, Ilya Sokolov wrote:
On Thu, 09 Sep 2010 18:25:57 -0700 Jeremy Maitin-Shepard<jeremy@jeremyms.com> wrote:
On 08/30/2010 11:59 AM, Boris Schaeling wrote:
[snip]
I checked now why I even had to add -lpthread if it wasn't required in earlier Boost.Process drafts: The new status class uses a worker thread to support asynchronous I/O. So yes, threading=multi is the natural choice. ;) Updated in SVN and in the ZIP file.
The use of a worker thread seems like an unfortunate additional burden. I don't know about the Windows implementation, but on Linux it seems in principle it should be possible to handle it via a SIGCHLD handler.
Callback would need to be async-signal-safe. If a user worries about performance so much, she could set up signal handler itself.
The user's own logic doesn't need to happen in the signal handler --- the signal handler can just take care of alerting asio to the event, so that it can then be processed later. Pipes are a standard way of implementing the communication. (Probably it would make sense to implement signal handling support as a general asio facility.) There is, of course, the general problem that setting a signal handler involves changing the global program state, and so it needs to be coordinated to some extent with the rest of the program. At the low level, it is probably useful for the user to be able to specify additional callback functions that will serve as additional signal handlers invoked by the boost-specified signal handler. Additionally, though, if there are a bunch of sub-processes, it would be inefficient to separately call waitpid for each of their pids in order to determine which, if any, caused the signal. It would be preferable, instead, to invoke waitpid/waitid with a pid of -1 to check all child processes. This, however, requires coordination with any other code that deals with child processes, since if waiting returns a pid not managed by boost process, some other code likely needs to be notified. Given the name boost.process, though, it doesn't seem unreasonable for the library to take responsibility for all of this. Obviously, this is a rather annoying complication compared to what is required for Windows, but it seems like the only efficient option.
On Sat, 11 Sep 2010 00:02:38 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]implementing the communication. (Probably it would make sense to implement signal handling support as a general asio facility.)
There is a Boost.Asio extension for signal handling in Trac: <https://svn.boost.org/trac/boost/ticket/2879> Actually there are two extensions. While mine follows Boost.Asio patterns more closely I remember that Dmitry pointed out some issues with the current implementation (see <http://thread.gmane.org/gmane.comp.lib.boost.devel/201581/focus=201923>). The main problem with his solution is that it doesn't support synchronous wait operations. Apart from that his implementation seems to be more lightweight. As I didn't look into the issues yet Dmitry had mentioned I wouldn't recommend anyone to use my Boost.Asio extension currently.
There is, of course, the general problem that setting a signal handler involves changing the global program state, and so it needs to be coordinated to some extent with the rest of the program. At the low
Exactly! As there also can be only one signal handler while you can have multiple Boost.Asio I/O service objects and I/O objects it gets even more complicated. As we felt it's more difficult to use libraries from different sources in one application when they all depend on SIGCHLD we went with wait(). You still need to coordinate. But this is something which we hope is easier to handle than libraries which steal signal handlers from each other. Boris
[...]
On 09/11/2010 10:12 AM, Boris Schaeling wrote:
On Sat, 11 Sep 2010 00:02:38 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]implementing the communication. (Probably it would make sense to implement signal handling support as a general asio facility.)
There is a Boost.Asio extension for signal handling in Trac: <https://svn.boost.org/trac/boost/ticket/2879>
Actually there are two extensions. While mine follows Boost.Asio patterns more closely I remember that Dmitry pointed out some issues with the current implementation (see <http://thread.gmane.org/gmane.comp.lib.boost.devel/201581/focus=201923>). The main problem with his solution is that it doesn't support synchronous wait operations. Apart from that his implementation seems to be more lightweight. As I didn't look into the issues yet Dmitry had mentioned I wouldn't recommend anyone to use my Boost.Asio extension currently.
Note that there are some additional non-POSIX facilities available that may be of use in implementing signal handling: - Since Linux kernel 2.6.22 and glibc 2.8, there is eventfd and signalfd. - On BSD (including Mac OS X), there is kqueue, which also allows receiving notification of signals. kqueue is particularly convenient because it allows signals to be monitored without disrupting their usual handling (and multiple queues can receive notification of the same signal even). Unfortunately, there doesn't seem to be any analogue on Linux that allows multiple independent notifications of a signal, even with signalfd. The problem with using any of these facilities is that it would increase the complexity of the overall code because there would be additional special cases.
There is, of course, the general problem that setting a signal handler involves changing the global program state, and so it needs to be coordinated to some extent with the rest of the program. At the low
Exactly! As there also can be only one signal handler while you can have multiple Boost.Asio I/O service objects and I/O objects it gets even more complicated. As we felt it's more difficult to use libraries from different sources in one application when they all depend on SIGCHLD we went with wait(). You still need to coordinate. But this is something which we hope is easier to handle than libraries which steal signal handlers from each other.
I agree that using a separate thread reduces the need to coordinate with other code, but it seems that having one additional thread per process that is launched is an unreasonably high cost. I don't really know what the best solution is.
On Sat, 11 Sep 2010 21:18:39 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]
Exactly! As there also can be only one signal handler while you can have multiple Boost.Asio I/O service objects and I/O objects it gets even more complicated. As we felt it's more difficult to use libraries from different sources in one application when they all depend on SIGCHLD we went with wait(). You still need to coordinate. But this is something which we hope is easier to handle than libraries which steal signal handlers from each other.
I agree that using a separate thread reduces the need to coordinate with other code, but it seems that having one additional thread per process that is launched is an unreasonably high cost.
Maybe I misunderstand what you mean with process. But you need (and should use) only one worker thread no matter for how many child processes you wait. Your code can look like this: boost::asio::io_service ioservice; boost::process::status s(ioservice); s.async_wait(child1.get_id(), handler); s.async_wait(child2.get_id(), handler); s.async_wait(child3.get_id(), handler); ... There is only one worker thread within the ioservice object. Boris
On 09/11/2010 01:47 PM, Boris Schaeling wrote:
Maybe I misunderstand what you mean with process. But you need (and should use) only one worker thread no matter for how many child processes you wait. Your code can look like this:
boost::asio::io_service ioservice; boost::process::status s(ioservice); s.async_wait(child1.get_id(), handler); s.async_wait(child2.get_id(), handler); s.async_wait(child3.get_id(), handler); ...
There is only one worker thread within the ioservice object.
I assumed you were using waitpid with the specific pid of the child, but I just took a look at the code and see that you are using wait (which waits for any child, equivalent to waitpid with a pid of -1). This indeed avoids the need for one thread per child process, but has the serious drawback that it will silently eat all notifications related to child processes not created by boost process, and therefore prevents any other code in the same program from properly dealing with child processes. Note that from a SIGCHLD handler, you could invoke waitpid with WNOHANG once for each child process, and this way avoid the need to coordinate regarding child process notification, but you would still need to coordinate regarding SIGCHLD, and I think it would be very inefficient to have to make one syscall per child process each time SIGCHLD arrives. If you arranged to have all of the child processes created by boost process actually be grandchildren of the main process, and children of a special forked process created by boost.process, then you could simultaneously avoid coordination regarding SIGCHLD and waitpid, at the cost of just one additional process. However, due to the need for an additional process, I don't really like this solution either.
On Sat, 11 Sep 2010 23:21:03 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]I assumed you were using waitpid with the specific pid of the child, but I just took a look at the code and see that you are using wait (which waits for any child, equivalent to waitpid with a pid of -1). This
Oh, sorry, it wasn't clear then.
indeed avoids the need for one thread per child process, but has the serious drawback that it will silently eat all notifications related to child processes not created by boost process, and therefore prevents any other code in the same program from properly dealing with child processes.
True. :(
[...]If you arranged to have all of the child processes created by boost process actually be grandchildren of the main process, and children of a special forked process created by boost.process, then you could simultaneously avoid coordination regarding SIGCHLD and waitpid, at the cost of just one additional process. However, due to the need for an additional process, I don't really like this solution either.
I have no better ideas either. It's really difficult to map the signal handler concept to Boost.Asio concepts. Maybe one shouldn't even try it and Dmitry's Boost.Asio extension is the better one. :) Boris
Jeremy Maitin-Shepard
On 09/11/2010 01:47 PM, Boris Schaeling wrote:
There is only one worker thread within the ioservice object.
I assumed you were using waitpid with the specific pid of the child, but I just took a look at the code and see that you are using wait (which waits for any child, equivalent to waitpid with a pid of -1). This indeed avoids the need for one thread per child process, but has the serious drawback that it will silently eat all notifications related to child processes not created by boost process, and therefore prevents any other code in the same program from properly dealing with child processes.
That's definitely bad behavior and must be avoided.
Note that from a SIGCHLD handler, you could invoke waitpid with WNOHANG once for each child process, and this way avoid the need to coordinate regarding child process notification, but you would still need to coordinate regarding SIGCHLD, and I think it would be very inefficient to have to make one syscall per child process each time SIGCHLD arrives.
Is not signal handling a process-specific behavior? It may be POSIX-specific, but it seems reasonable that Boost.Process provide signal handling support. Then, Boost.Process can require that all code in a process using that library must coordinate with its signal handling. If Boost.Process provides useful and convenient support for signal handling, most users of the library would use that for their signal handling. Those that must integrate with other signal handling code need only have some means of tying both together. I have code for doing such convenient signal handling that I think I could get permission to share if you're interested. It would, at least, be a good jump start.
If you arranged to have all of the child processes created by boost process actually be grandchildren of the main process, and children of a special forked process created by boost.process, then you could simultaneously avoid coordination regarding SIGCHLD and waitpid, at the cost of just one additional process. However, due to the need for an additional process, I don't really like this solution either.
That it behaves properly is an excellent recommendation for this approach compared with wait(). I'm not sure that the overhead of yet another process is an insuperable problem, but it is cause for concern. Still, if Boost.Process takes on signal handling and requires that other signal handling code cooperate with it, then SIGCHLD is made available and eliminates the need for the interstitial process. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 09/13/2010 04:41 AM, Stewart, Robert wrote:
[snip]
Is not signal handling a process-specific behavior? It may be POSIX-specific, but it seems reasonable that Boost.Process provide signal handling support. Then, Boost.Process can require that all code in a process using that library must coordinate with its signal handling.
I would not say that general signal handling falls within the domain of Boost.Process, but it may make sense for Boost.Process to depend on a not-yet-existent Boost POSIX Signal Handling library. Note, however, that coordination solely at the level of signals is not sufficient, at least if the code is to scale to many child processes. Rather, specific coordination with respect to SIGCHLD and waitpid is needed.
[snip]
Jeremy Maitin-Shepard wrote:
On 09/13/2010 04:41 AM, Stewart, Robert wrote:
Is not signal handling a process-specific behavior? It may be POSIX-specific, but it seems reasonable that Boost.Process provide signal handling support. Then, Boost.Process can require that all code in a process using that library must coordinate with its signal handling.
I would not say that general signal handling falls within the domain of Boost.Process, but it may make sense for Boost.Process to depend on a not-yet-existent Boost POSIX Signal Handling library.
I have no problem with that.
Note, however, that coordination solely at the level of signals is not sufficient, at least if the code is to scale to many child processes. Rather, specific coordination with respect to SIGCHLD and waitpid is needed.
Hmmm. As I see it, Boost.Process would know which child processes it created and would only call waitpid() on those. No code outside the library would need to call waitpid() on those. All that's needed is for Boost.Process' SIGCHLD handler to be invoked and, if the other code needs a SIGCHLD handler, that it be invoked by the same mechanism, to ensure correct chaining, and that the other code never calls waitpid() for child processes that Boost.Process creates. Is that what you meant by "specific coordination?" That's not coordination so much as clear delineation of responsibilities and behaviors, but it would mean we were just talking past one another. If you meant something else, please clarify. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 09/13/2010 11:15 AM, Stewart, Robert wrote:
[snip]
Hmmm. As I see it, Boost.Process would know which child processes it created and would only call waitpid() on those. No code outside the library would need to call waitpid() on those. All that's needed is for Boost.Process' SIGCHLD handler to be invoked and, if the other code needs a SIGCHLD handler, that it be invoked by the same mechanism, to ensure correct chaining, and that the other code never calls waitpid() for child processes that Boost.Process creates.
Is that what you meant by "specific coordination?" That's not coordination so much as clear delineation of responsibilities and behaviors, but it would mean we were just talking past one another. If you meant something else, please clarify.
If Boost.Process has created multiple not-yet-terminated child processes, you would need to call waitpid once for each child every time SIGCHLD arrives. This would not scale well to e.g. 200 child processes. Likewise, you cannot expect code outside of Boost.Process that deals with child processes to have to invoke waitpid in a polling fashion. (I don't know what the actual overhead cost is, but in any case it doesn't seem reasonable.)
Jeremy Maitin-Shepard wrote:
On 09/13/2010 11:15 AM, Stewart, Robert wrote:
Hmmm. As I see it, Boost.Process would know which child processes it created and would only call waitpid() on those. No code outside the library would need to call waitpid() on those. All that's needed is for Boost.Process' SIGCHLD handler to be invoked and, if the other code needs a SIGCHLD handler, that it be invoked by the same mechanism, to ensure correct chaining, and that the other code never calls waitpid() for child processes that Boost.Process creates.
Is that what you meant by "specific coordination?" That's not coordination so much as clear delineation of responsibilities and behaviors, but it would mean we were just talking past one another. If you meant something else, please clarify.
If Boost.Process has created multiple not-yet-terminated child processes, you would need to call waitpid once for each child every time SIGCHLD arrives. This would not scale well to e.g. 200 child processes.
Likewise, you cannot expect code outside of Boost.Process that deals with child processes to have to invoke waitpid in a polling fashion. (I don't know what the actual overhead cost is, but in any case it doesn't seem reasonable.)
Indeed, calling waitpid() for each child, in turn, is not a good idea from a scalability perspective. Putting all of Boost.Process' children into a unique process group and using waitpid(), with WNOHANG, to wait for children in that process group could work though. If the client can control the process group of any child created with Boost.Process, then the library can track the process groups that have been specified and call waitpid() for each in turn. If the PID of the terminated child is not recognized, then, it must have been created by other code in the process using one of the process groups given to the library. In that case, a callback function would permit the library to inform the client that such a child terminated. That doesn't help when the client cannot communicate such information to a third-party library that wants SIGCHLD. Sadly, even chaining SIGCHLD handlers won't help in that case. Here's another idea: if Boost.Process managed one child process that itself actually created the children, and communicated their creation and demise via pipe or socket, then that process can use a SIGCHLD handler with impunity and Boost.Process, in the grandparent process, will only be informed of the demise of grandchildren created by the library. Since creating a child process to manage grandchild processes is heavier weight than the process group approach, but the latter won't always work, perhaps the library could offer both options. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 09/14/2010 04:04 AM, Stewart, Robert wrote:
[snip]
Indeed, calling waitpid() for each child, in turn, is not a good idea from a scalability perspective. Putting all of Boost.Process' children into a unique process group and using waitpid(), with WNOHANG, to wait for children in that process group could work though.
The process group approach does have some nice properties, but I'm not sure it is reasonable for the library to impose such a restriction. (Maybe it is, I am simply not sufficiently familiar with process groups and their intended purpose/practical use to know.) One thing of note is that a process can change its own process group id.
If the client can control the process group of any child created with Boost.Process, then the library can track the process groups that have been specified and call waitpid() for each in turn. If the PID of the terminated child is not recognized, then, it must have been created by other code in the process using one of the process groups given to the library. In that case, a callback function would permit the library to inform the client that such a child terminated. That doesn't help when the client cannot communicate such information to a third-party library that wants SIGCHLD. Sadly, even chaining SIGCHLD handlers won't help in that case.
To some extent, any library that doesn't provide such an interface is inherently broken in terms of interoperability. The C system() function would still work as advertised, since it blocks SIGCHLD, but due to that blocking, should probably be avoided in many cases.
Here's another idea: if Boost.Process managed one child process that itself actually created the children, and communicated their creation and demise via pipe or socket, then that process can use a SIGCHLD handler with impunity and Boost.Process, in the grandparent process, will only be informed of the demise of grandchildren created by the library.
Since creating a child process to manage grandchild processes is heavier weight than the process group approach, but the latter won't always work, perhaps the library could offer both options.
Jeremy Maitin-Shepard
On 09/14/2010 04:04 AM, Stewart, Robert wrote:
Indeed, calling waitpid() for each child, in turn, is not a good idea from a scalability perspective. Putting all of Boost.Process' children into a unique process group and using waitpid(), with WNOHANG, to wait for children in that process group could work though.
The process group approach does have some nice properties, but I'm not sure it is reasonable for the library to impose such a restriction. (Maybe it is, I am simply not sufficiently familiar with process groups and their intended purpose/practical use to know.) One thing of note is that a process can change its own process group id.
From Wikipedia: "Process groups are used to control the distribution of signals."
The idea is that you can send a signal to all members of a process group, but using that to monitor a signal from any member of the group via waitpid() seems quit within scope, particularly since that is part of the waitpid() interface. I'm also not familiar with the behaviors and uses of process groups so I don't know if this would just shift the problem in some way, albeit to presumably less common application types. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 09/14/2010 10:58 AM, Stewart, Robert wrote:
Jeremy Maitin-Shepard
On 09/14/2010 04:04 AM, Stewart, Robert wrote:
Indeed, calling waitpid() for each child, in turn, is not a good idea from a scalability perspective. Putting all of Boost.Process' children into a unique process group and using waitpid(), with WNOHANG, to wait for children in that process group could work though.
The process group approach does have some nice properties, but I'm not sure it is reasonable for the library to impose such a restriction. (Maybe it is, I am simply not sufficiently familiar with process groups and their intended purpose/practical use to know.) One thing of note is that a process can change its own process group id.
From Wikipedia: "Process groups are used to control the distribution of signals."
The idea is that you can send a signal to all members of a process group, but using that to monitor a signal from any member of the group via waitpid() seems quit within scope, particularly since that is part of the waitpid() interface.
I'm also not familiar with the behaviors and uses of process groups so I don't know if this would just shift the problem in some way, albeit to presumably less common application types.
In addition to the fact that you could still run into the same problem if you allow users to change process groups (you would have to loop through arbitrarily many process groups), I feel that imposing non-standard restrictions, even if seemingly minor, is not really appropriate for a Boost Process library. I took a look at the process spawning facilities in GLib and Qt (documentation only). There might be other libraries that would also be relevant. See: http://library.gnome.org/devel/glib/2.25/glib-Spawning-Processes.html http://library.gnome.org/devel/glib/2.25/glib-The-Main-Event-Loop.html Note: The Main Loop documentation describes the "child watch" facilities, which are particularly relevant to our discussion. http://doc.qt.nokia.com/4.7-snapshot/qprocess.html The GLib approach in particular seems to be most relevant. (Qt, I believe, tends to assume it has full control over the entire process, whereas Glib seems to attempt to be more modular and interoperable with other software, at least with respect to process creation.) It seems a key goal is for Boost Process to be interoperable with the process creation in GLib and Qt. GLib may also provide some useful design inspiration.
Jeremy Maitin-Shepard wrote:
On 09/14/2010 10:58 AM, Stewart, Robert wrote:
Jeremy Maitin-Shepard
On 09/14/2010 04:04 AM, Stewart, Robert wrote:
Indeed, calling waitpid() for each child, in turn, is not a good idea from a scalability perspective. Putting all of Boost.Process' children into a unique process group and using waitpid(), with WNOHANG, to wait for children in that process group could work though.
The process group approach does have some nice properties, but I'm not sure it is reasonable for the library to impose such a restriction. (Maybe it is, I am simply not sufficiently familiar with process groups and their intended purpose/practical use to know.) One thing of note is that a process can change its own process group id.
From Wikipedia: "Process groups are used to control the distribution of signals."
The idea is that you can send a signal to all members of a process group, but using that to monitor a signal from any member of the group via waitpid() seems quit within scope, particularly since that is part of the waitpid() interface.
I'm also not familiar with the behaviors and uses of process groups so I don't know if this would just shift the problem in some way, albeit to presumably less common application types.
In addition to the fact that you could still run into the same problem if you allow users to change process groups (you would have to loop through arbitrarily many process groups), I feel that imposing non-standard restrictions, even if seemingly minor, is not really appropriate for a Boost Process library.
I've done some more reading on the subject. According to [1], the purpose of process groups is, as suggested by, but not stated in, Wikipedia [2], for shells to manage processes associated with a tty. However, [3] notes the reason for creating a process group, in the context of the tutorial at least, is so "one may kill all the processes in the process group without having to keep track of how many processes have been forked and all of their process id's." Of course, "kill" can mean send any signal, not just SIGTERM or SIGKILL, for example. Note that Boost.Process would need to set the process group for each child and would likely use the first child's PID as the process group ID. Since the parent would not be part of the new process group and the children would inherit the parent's session, the process group won't be orphaned unless the parent process dies. In that case, the children will get SIGHUP followed by SIGCONT to alert them to the loss of the controlling process; that should be documented so library clients can consider handling that case (likely by ignoring SIGHUP). See [5] for more on this. Aside from wanting to signal all child processes, I consider it to be unusual for an application to set its process group and much more so for a client of Boost.Process to do so [4]. A Boost.Process client would only want to set the process group of its children to permit the use of kill() to signal them all at once. If there's a mechanism for doing that on Windows, the library would need to provide a portable means to do so, and that means clients wouldn't need to rely on kill() and a process group to support it. Consequently, Boost.Process could preclude the manipulation of process groups and using kill() on child processes via a process group.
I took a look at the process spawning facilities in GLib and Qt (documentation only). There might be other libraries that would also be relevant. See:
<http://library.gnome.org/devel/glib/2.25/glib-Spawning-Processes.html> <http://library.gnome.org/devel/glib/2.25/glib-The-Main-Event-Loop.html>
Note: The Main Loop documentation describes the "child watch" facilities, which are particularly relevant to our discussion.
I found little of use there except that they disallow calling waitpid(-1); did I miss something important? Using a process group means there's no need to do that anyway.
It seems a key goal is for Boost Process to be interoperable with the process creation in GLib and Qt. GLib may also provide some useful design inspiration.
I agree on both counts. Avoiding waitpid(-1) seems necessary for this and reasons you've stated before. Using waitpid(-pgid), however, would fit nicely. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com [1] <http://www.cis.temple.edu/~ingargio/old/cis307s96/readings/docs/signals.html#Pgrps> [2] <http://en.wikipedia.org/wiki/Process_group> [3] <http://www.yolinux.com/TUTORIALS/ForkExecProcesses.html>, see section entitled, "Kill all processes in a process group" [4] A client of Boost.Process, being somewhat hampered by the restraints of portability, would necessarily not have as complex process management needs as a shell, unless I'm much mistaken. [5] <http://www.win.tue.nl/~aeb/linux/lk/lk-10.html#ss10.2> IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Hi, I ran into a problem when using the 2010 boost.process library within a Windows GUI application with WinMain() instead of main() without a console. When I construct a context object the std[in|out|err]_behavior members are initialized in the constructor with calls to behavior::inherit::create(GetStdHandle(...)), which calls DuplicateHandle(). DuplicateHandle() fails with error 6 (invalid handle) since the application has no console and an exception is thrown. So currently I don't see how I could use a context object from within a Windows GUI application since its constructor will always throw this exception. What I want to do is start a child process and read its stdout as in the example "Communicating with child processes". Best regards, Wolfgang
Wolfgang Fertsak wrote:
Hi, I ran into a problem when using the 2010 boost.process library within a Windows GUI application with WinMain() instead of main() without a console.
When I construct a context object the std[in|out|err]_behavior members are initialized in the constructor with calls to behavior::inherit::create(GetStdHandle(...)), which calls DuplicateHandle(). DuplicateHandle() fails with error 6 (invalid handle) since the application has no console and an exception is thrown.
So currently I don't see how I could use a context object from within a Windows GUI application since its constructor will always throw this exception.
What I want to do is start a child process and read its stdout as in the example "Communicating with child processes".
I've run into the same problem. It points out, IMHO, a major weakness in the design of the fundamental classes, which requires all context's to provide stream behaviors. To work around this issue I needed to create a null stream behavior, and a nullio_context. As in: namespace boost { namespace process { namespace behavior { class null : public stream { public: static boost::shared_ptr<null> create() { return boost::make_shared<null>(); } }; }}} namespace boost { namespace process { struct nullio_context { boost::shared_ptr<boost::process::behavior::stream> stdin_behavior; boost::shared_ptr<boost::process::behavior::stream> stdout_behavior; boost::shared_ptr<boost::process::behavior::stream> stderr_behavior; std::string process_name; std::string work_dir; boost::process::environment env; nullio_context() : stdin_behavior(boost::process::behavior::null::create()) , stdout_behavior(boost::process::behavior::null::create()) , stderr_behavior(boost::process::behavior::null::create()) , work_dir(boost::process::self::get_work_dir()) , env(boost::process::self::get_environment()) {} #if defined(BOOST_POSIX_API) void setup(std::vector<bool> &closeflags) { } #elif defined(BOOST_WINDOWS_API) void setup(STARTUPINFOA& ) { } #endif }; }} These are required because create_child reaches into context and asserts that there are instances of std in/out/err. It then directly accesses each of these I initially tried using the provided dummy behavior, which is a misnomer since it actually created temporary files and streams. I was an unhandled win32 exception when shutting down. The funny thing is there is already a context::setup method that would - at least on windows - allow the stream behavior to be fully encapsulated. A context could set the handle members of STARTUPINFOA only instantiating those boost::process::behavior::streams that are needed. On posix, setup could do all of the dup'ing of filenumbers, as it's called between fork and execve already. I've got a feeling there is a much better design lurking in the existing one, that would give a much more natural abstraction of the posix/windows requirements. Jeff
On 31.08.2010 15:08, Jeff Flinn wrote:
[...] I initially tried using the provided dummy behavior, which is a misnomer since it actually created temporary files and streams. I was an unhandled win32 exception when shutting down.
The funny thing is there is already a context::setup method that would - at least on windows - allow the stream behavior to be fully encapsulated. A context could set the handle members of STARTUPINFOA only instantiating those boost::process::behavior::streams that are needed. On posix, setup could do all of the dup'ing of filenumbers, as it's called between fork and execve already.
I've got a feeling there is a much better design lurking in the existing one, that would give a much more natural abstraction of the posix/windows requirements.
Thanks for the tip with the custom context class! (I think class null in your example is not neccessary because you could use boost::process::behavior::stream::create() directly instead). IMO it would be nice if the library provided some support for our problem, e.g. by an additional constructor for the context class where you can explicitly specify values for each of the members (perhaps with default arguments). Best regards, Wolfgang
On Tue, 31 Aug 2010 15:08:37 +0200, Jeff Flinn <TriumphSprint2000@hotmail.com> wrote:
[...]I initially tried using the provided dummy behavior, which is a misnomer since it actually created temporary files and streams. I was an unhandled win32 exception when shutting down.
What name would you prefer (instead of dummy)?
The funny thing is there is already a context::setup method that would - at least on windows - allow the stream behavior to be fully encapsulated. A context could set the handle members of STARTUPINFOA only instantiating those boost::process::behavior::streams that are needed. On posix, setup could do all of the dup'ing of filenumbers, as it's called between fork and execve already.
I'm not sure if I understand correctly: If setup() is required to setup the STARTUPINFOA structure on Windows and to duplicate or close file descriptors on POSIX why using a library at all? Actually create_child() and stream behaviors do all of this - why would you want to do this yourself in setup()? If the only problem is that DuplicateHandle() must not be called in Windows applications we could change the standard constructor of context. In fact in previous Boost.Process drafts standard streams were not inherited but closed - but then POSIX developers complain again as they expect inheritance. Wolfgang proposed to add another constructor. In order to avoid that Windows applications accidentally use the default constructor we could also provide only one constructor with three required parameters to set stream behaviors? Or another idea: Boost.Process does whatever developers expect on a platform (inherit on POSIX, nothing on Windows)? Boris
On 01.09.2010 01:03, Boris Schaeling wrote:
[...] In fact in previous Boost.Process drafts standard streams were not inherited but closed - but then POSIX developers complain again as they expect inheritance. Wolfgang proposed to add another constructor. In order to avoid that Windows applications accidentally use the default constructor we could also provide only one constructor with three required parameters to set stream behaviors? Or another idea: Boost.Process does whatever developers expect on a platform (inherit on POSIX, nothing on Windows)?
I think the current implementation can cause problems on POSIX too. When an executables closes its stdin/out/err file descriptors (e.g. a demon), inheriting these handles will also fail (and I guess exceptions will be thrown). So regardless of what the default behavior is, POSIX and Windows developers should be able to change it. IMO another constructor with the 3 parameters for the stream behaviours would do. The documentation should state clearly that the default behavior is inheritance and will fail if the application has no console or its stdin/out/err file descriptors have been closed. But that's just my 5 cents...other opinions/ideas would be nice. Wolfgang
Wolfgang Fertsak wrote:
On 01.09.2010 01:03, Boris Schaeling wrote:
I think the current implementation can cause problems on POSIX too. When an executables closes its stdin/out/err file descriptors (e.g. a demon), inheriting these handles will also fail (and I guess exceptions will be thrown). So regardless of what the default behavior is, POSIX and Windows developers should be able to change it. IMO another constructor with the 3 parameters for the stream behaviours would do. The documentation should state clearly that the default behavior is inheritance and will fail if the application has no console or its stdin/out/err file descriptors have been closed.
Here are a couple of thoughts on how you might solve the dilemma: Try the Named Constructor Idiom. One can choose which behavior one wants by invoking a static member function with a suggestive name and requiring whatever arguments are appropriate and returning an instance of the class. That's often clearer than overloading constructors. If it is possible to determine whether a Windows app is being built as a console app versus a GUI app, then you could conditionally compile for one or the other, thus keeping a Windows developer from choosing incompatible behavior. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 01.09.2010 14:37, Stewart, Robert wrote:
Here are a couple of thoughts on how you might solve the dilemma:
Try the Named Constructor Idiom. One can choose which behavior one wants by invoking a static member function with a suggestive name and requiring whatever arguments are appropriate and returning an instance of the class. That's often clearer than overloading constructors.
If it is possible to determine whether a Windows app is being built as a console app versus a GUI app, then you could conditionally compile for one or the other, thus keeping a Windows developer from choosing incompatible behavior.
Sounds good, thanks! Wolfgang
On Thu, 02 Sep 2010 08:59:02 +0200, Wolfgang Fertsak <wolfgang.fertsak@allianz.at> wrote:
On 01.09.2010 14:37, Stewart, Robert wrote:
Here are a couple of thoughts on how you might solve the dilemma:
Try the Named Constructor Idiom. One can choose which behavior one wants by invoking a static member function with a suggestive name and requiring whatever arguments are appropriate and returning an instance of the class. That's often clearer than overloading constructors.
If it is possible to determine whether a Windows app is being built as a console app versus a GUI app, then you could conditionally compile for one or the other, thus keeping a Windows developer from choosing incompatible behavior.
Sounds good, thanks!
What exactly sounds good? The first or second proposal? :-) I think we should go with the first one. Adapting the standard behavior to a platform is nice, too. However I'm a bit worried that developers expect a consistent behavior across all platforms. Boris
On 02.09.2010 23:39, Boris Schaeling wrote:
What exactly sounds good? The first or second proposal? :-)
I think we should go with the first one. Adapting the standard behavior to a platform is nice, too. However I'm a bit worried that developers expect a consistent behavior across all platforms.
Boris
Sorry...I also meant the first one - consistency is more important! "Guessing" what kind of application is being built sounds a bit too "adventurous" :-) Wolfgang
On Fri, 03 Sep 2010 14:58:10 +0200, Wolfgang Fertsak <wolfgang.fertsak@allianz.at> wrote:
On 02.09.2010 23:39, Boris Schaeling wrote:
What exactly sounds good? The first or second proposal? :-)
I think we should go with the first one. Adapting the standard behavior to a platform is nice, too. However I'm a bit worried that developers expect a consistent behavior across all platforms.
Boris
Sorry...I also meant the first one - consistency is more important! "Guessing" what kind of application is being built sounds a bit too "adventurous" :-)
The default behavior of a context class should still be inheriting standard streams though? Given that there are countless combinations of stream behaviors possible we either provide a default behavior or require developers to explicitly set stream behaviors for all standard streams? Boris
On 04.09.2010 00:26, Boris Schaeling wrote:
The default behavior of a context class should still be inheriting standard streams though? Given that there are countless combinations of stream behaviors possible we either provide a default behavior or require developers to explicitly set stream behaviors for all standard streams?
Inheritance as default behavior is fine with me... Wolfgang
Boris Schaeling wrote:
On Tue, 31 Aug 2010 15:08:37 +0200, Jeff Flinn wrote:
[...]I initially tried using the provided dummy behavior, which is a misnomer since it actually created temporary files and streams. I was an unhandled win32 exception when shutting down.
What name would you prefer (instead of dummy)?
I'd rather know how Jeff became a WIN32 exception and why he was shutting down! ;-) Seriously, the more important question is why the dummy class caused the creation of temporaries. It would seem there should be no such side effects as the name suggests NOP. If those side effects can be eliminated, then "null" would be a good component of the name. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Stewart, Robert wrote:
Boris Schaeling wrote:
On Tue, 31 Aug 2010 15:08:37 +0200, Jeff Flinn wrote:
[...]I initially tried using the provided dummy behavior, which is a misnomer since it actually created temporary files and streams. I was an unhandled win32 exception when shutting down. What name would you prefer (instead of dummy)?
I'd rather know how Jeff became a WIN32 exception and why he was shutting down! ;-)
LOL, personal family history aside, I actually meant to type: An unhandled win32 exception was throw from by the child process when it was shutting down.
Seriously, the more important question is why the dummy class caused the creation of temporaries. It would seem there should be no such side effects as the name suggests NOP. If those side effects can be eliminated, then "null" would be a good component of the name.
Agreed. Jeff
On Wed, 01 Sep 2010 15:14:43 +0200, Jeff Flinn <TriumphSprint2000@hotmail.com> wrote:
[...]
Seriously, the more important question is why the dummy class caused the creation of temporaries. It would seem there should be no such side effects as the name suggests NOP. If those side effects can be eliminated, then "null" would be a good component of the name.
Agreed.
Changed. Boris
On 01.09.2010 3:03, Boris Schaeling wrote:
On Tue, 31 Aug 2010 15:08:37 +0200, Jeff Flinn <TriumphSprint2000@hotmail.com> wrote:
[...]I initially tried using the provided dummy behavior, which is a misnomer since it actually created temporary files and streams. I was an unhandled win32 exception when shutting down.
What name would you prefer (instead of dummy)?
The obvious name for dummy is null or the like, as it redirects to/from /dev/null, /dev/zero or NUL.
The funny thing is there is already a context::setup method that would - at least on windows - allow the stream behavior to be fully encapsulated. A context could set the handle members of STARTUPINFOA only instantiating those boost::process::behavior::streams that are needed. On posix, setup could do all of the dup'ing of filenumbers, as it's called between fork and execve already.
I'm not sure if I understand correctly: If setup() is required to setup the STARTUPINFOA structure on Windows and to duplicate or close file descriptors on POSIX why using a library at all? Actually create_child() and stream behaviors do all of this - why would you want to do this yourself in setup()? If the only problem is that DuplicateHandle() must not be called in Windows applications we could change the standard constructor of context. In fact in previous Boost.Process drafts standard streams were not inherited but closed - but then POSIX developers complain again as they expect inheritance. Wolfgang proposed to add another constructor. In order to avoid that Windows applications accidentally use the default constructor we could also provide only one constructor with three required parameters to set stream behaviors? Or another idea: Boost.Process does whatever developers expect on a platform (inherit on POSIX, nothing on Windows)?
Boris
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
As there has been a lot of feedback I started to update the library. There is now a new version available at <http://www.highscore.de/boost/gsoc2010/process.zip> (and of course in Subversion at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/>). What has been changed? * The context class now uses delegates based on boost::function to configure standard streams. There is no class hierarchy anymore for stream behavior classes. Nothing needs to be created on the heap either (and consequently boost::shared_ptr is gone). Context objects can be reused now to create multiple child processes. * The delegates in the context class expect this signature: std::pair<handle, handle> (bool). There is a parameter of type bool passed to indicate whether an input or an output stream is configured. Now it's not required anymore to pass a parameter like input_stream or output_stream yourself. * All stream behaviors have been defined as function objects. While it would be possible to create standalone functions for some stream behaviors (like behavior::close which would return a pair of empty handles) for consistency I defined function objects only. * behavior::named_pipe now requires the developer to set the name of the pipe (and does not automatically generate a unique name anymore). * behavior::async_pipe is a new stream behavior which uses the minimum required pipe type on a platform to support asynchronous I/O. It's a typedef for (anonmyous) pipe on POSIX and a function object based on behavior::named_pipe on Windows (automatically generating a unique name with UuidCreateSequential()). I'd appreciate if you could comment on the changes. Especially have a look at: * this signature: std::pair<handle, handle> (bool). Shall we use std::pair<handle, handle> to return two handles for the child and parent process or a struct with member variables called parent_end and child_end? Shall we use a bool to indicate whether an input or output stream is configured or something else like an enumeration? I think it all depends on how many developers want to define new stream behaviors and if it's worth to make the signature a bit more self-explanatory? * the implementation of async_pipe on Windows. Are return values of Windows functions checked correctly? Is the return value of UuidCreateSequential() and UuidToStringA() the error code or must GetLastError() be called? Is it OK if we require developers to link against rpcrt4.lib on Windows because async_pipe uses UuidCreateSequential() now? * the example at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/redirect_to.cpp>. It uses a user-defined behavior class to redirect the child's standard error stream to the standard output stream. As the behavior of the standard error stream must somehow access the handle of the standard output stream I create the pipe myself before I set the stream behaviors. As the pipe must be used for the standard output stream though I define a forward function which I can bind to the respective delegate. Anyway, please have a look at the example and think about whether it makes sense or if there is anything to simplify. :-) What still has to be done? * The context class still has a default constructor only which initializes the stream behaviors to inherit standard streams. This is a problem for Windows GUI applications as they don't have standard streams (or at least they are closed). While the Named Constructor Idiom has been proposed what exactly should the done? Should the default constructor be private and developers must explicitly use a factory function to create a context object? Shall the factory function initialize all standard streams with the same behavior? Shall we provide a factory function which expects three parameters to initialize the standard streams? What's convenient and flexible and not too confusing? * I didn't get rid yet of boost::filesystem::filesystem_error, boost::algorithm::ends_with and boost::algorithm::iends_with in operations.hpp. I read Ilya's posting and understand that he is worried about including too much. I checked Boost.Filesystem's header files and saw that filesystem/exception.hpp is empty and the exception classes have been moved to filesystem/path.hpp. I don't know the reason but wonder whether this shouldn't be changed then in Boost.Filesystem. If a Boost library can't include a header file from another Boost library because the header file is too big the header file should be split? Boost.StringAlgorithms is better in this regard (and I included only boost/algorithm/string/predicate.hpp and not everything from Boost.StringAlgorithms; of course this header file includes other header files from Boost.StringAlgorithms again...). Anyway, if anyone wants to reimplement find_executable_in_path() and executable_to_progname() (both in operations.hpp) I'm fine with it. I'm honestly not that motivated to reinvent the wheel and hope I can spend my time doing something more productive. * The documentation must be updated (which I'll do when everyone agrees that the changes make sense). Boris
Boris Schaeling wrote:
* The delegates in the context class expect this signature: std::pair<handle, handle> (bool). There is a parameter of type bool passed to indicate whether an input or an output stream is configured. Now it's not required anymore to pass a parameter like input_stream or output_stream yourself.
Why is true input and false ouput (or the reverse)? Use an enumerated type for clarity.
* this signature: std::pair<handle, handle> (bool). Shall we use std::pair<handle, handle> to return two handles for the child and parent process or a struct with member variables called parent_end and child_end?
A child naturally ensues from a parent, so the order parent then child is easily remembered, though there is no harm in creating a struct that makes the members explicit.
Shall we use a bool to indicate whether an input or output stream is configured or something else like an enumeration? I think it all depends on how many developers want to define new stream behaviors and if it's worth to make the signature a bit more self-explanatory?
bool is never a good idea for such arguments, regardless of the possibility of extending the set. ___ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Mon, 06 Sep 2010 03:07:26 +0200, Stewart, Robert <Robert.Stewart@sig.com> wrote:
[...]
* this signature: std::pair<handle, handle> (bool). Shall we use std::pair<handle, handle> to return two handles for the child and parent process or a struct with member variables called parent_end and child_end?
A child naturally ensues from a parent, so the order parent then child is easily remembered, though there is no harm in creating a struct that makes the members explicit.
Well, as stream behaviors are all about creating child processes I decided to store the child's handle in the "first" member variable of std::pair. So much about what people think is natural. :)
Shall we use a bool to indicate whether an input or output stream is configured or something else like an enumeration? I think it all depends on how many developers want to define new stream behaviors and if it's worth to make the signature a bit more self-explanatory?
bool is never a good idea for such arguments, regardless of the possibility of extending the set.
It smells a bit like over-engineering to me but then I don't really mind to change it. More opinions? Boris
Boris Schaeling wrote:
On Mon, 06 Sep 2010 03:07:26 +0200, Stewart, Robert <Robert.Stewart@sig.com> wrote:
Shall we use a bool to indicate whether an input or output stream is configured or something else like an enumeration? I think it all depends on how many developers want to define new stream behaviors and if it's worth to make the signature a bit more self-explanatory?
bool is never a good idea for such arguments, regardless of the possibility of extending the set.
It smells a bit like over-engineering to me but then I don't really mind to change it. More opinions?
As I noted in the snipped portion of my reply, why would anyone associate true (or false) with input and the other with output? bools are great when a value or argument is, itself, a Boolean quantity, but when they are used to represent non-Boolean quantities, there is no association possible beyond rote memorization. Besides, removing doubt and adding clarity is hardly over-engineering. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Tue, 07 Sep 2010 12:45:27 +0200, Stewart, Robert <Robert.Stewart@sig.com> wrote:
Boris Schaeling wrote:
On Mon, 06 Sep 2010 03:07:26 +0200, Stewart, Robert <Robert.Stewart@sig.com> wrote:
Shall we use a bool to indicate whether an input or output stream is configured or something else like an enumeration? I think it all depends on how many developers want to define new stream behaviors and if it's worth to make the signature a bit more self-explanatory?
bool is never a good idea for such arguments, regardless of the possibility of extending the set.
It smells a bit like over-engineering to me but then I don't really mind to change it. More opinions?
As I noted in the snipped portion of my reply, why would anyone associate true (or false) with input and the other with output? bools are great when a value or argument is, itself, a Boolean quantity, but when they are used to represent non-Boolean quantities, there is no association possible beyond rote memorization.
Besides, removing doubt and adding clarity is hardly over-engineering.
We are focusing here on a bool variable 99% of library users will never see. And I don't think the other 1% will need more than 5 seconds to find out what it does. But as I said I don't mind to change it. And if this is the only issue the library seems to be in a pretty good shape now. ;) I'll update the code and documentation in the coming days. As discussions seem to have calmed down a bit I'll ask then for a review. Thanks for everyone's feedback so far! Boris
On Sun, 05 Sep 2010 18:05:36 +0400, Boris Schaeling <boris@highscore.de> wrote:
As there has been a lot of feedback I started to update the library. There is now a new version available at <http://www.highscore.de/boost/gsoc2010/process.zip> (and of course in Subversion at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/>).
What has been changed?
[snip]
I'd appreciate if you could comment on the changes. Especially have a look at:
* this signature: std::pair<handle, handle> (bool).
It would be better if delegate would accept fileno instead of bool indicating input/output. bp::behavior::inherit seems almost indistinguishable from the redirect_to.
Shall we use std::pair<handle, handle> to return two handles for the child and parent process or a struct with member variables called parent_end and child_end? Shall we use a bool to indicate whether an input or output stream is configured or something else like an enumeration? I think it all depends on how many developers want to define new stream behaviors and if it's worth to make the signature a bit more self-explanatory?
* the implementation of async_pipe on Windows. Are return values of Windows functions checked correctly?
Nod.
Is the return value of UuidCreateSequential() and UuidToStringA() the error code or must GetLastError() be called?
The former, and it should be passed to the system_error's constructor. system_category should be fine for it.
Is it OK if we require developers to link against rpcrt4.lib on Windows because async_pipe uses UuidCreateSequential() now?
Nod.
* the example at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/redirect_to.cpp>. It uses a user-defined behavior class to redirect the child's standard error stream to the standard output stream. As the behavior of the standard error stream must somehow access the handle of the standard output stream I create the pipe myself before I set the stream behaviors. As the pipe must be used for the standard output stream though I define a forward function which I can bind to the respective delegate. Anyway, please have a look at the example and think about whether it makes sense or if there is anything to simplify. :-)
IMO, the redirect_to_stderr behavior must be provided by the library as it is a common use case. Unfortunately, it breaks the interface (((.
What still has to be done?
* The context class still has a default constructor only which initializes the stream behaviors to inherit standard streams. This is a problem for Windows GUI applications as they don't have standard streams (or at least they are closed). While the Named Constructor Idiom has been proposed what exactly should the done? Should the default constructor be private and developers must explicitly use a factory function to create a context object? Shall the factory function initialize all standard streams with the same behavior? Shall we provide a factory function which expects three parameters to initialize the standard streams? What's convenient and flexible and not too confusing?
I expect that if some std stream is closed in the calling process, the default constructor of the context class doesn't throw anything. As I see it, the bp::handle should have a "dont-close" flag and bp::behavior::inherit shouldn't call dup() or DuplicateHandle().
[snip]
On Wed, 08 Sep 2010 03:37:52 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]
I'd appreciate if you could comment on the changes. Especially have a look at:
* this signature: std::pair<handle, handle> (bool).
It would be better if delegate would accept fileno instead of bool indicating input/output. bp::behavior::inherit seems almost indistinguishable from the redirect_to.
Which fileno should be passed? stdin/stdout/stderr of the parent process? Maybe we have a similar idea because I also think it would be better to distinguish standard output and standard error. This could be done with an enumeration and could maybe help to create a better redirect_to stream behavior. The problem with redirect_to though is that it depends on another stream behavior. I don't know whether Boost.Process should support dependencies between stream behaviors? Here is some pseudocode which could work somehow: ---------- enum stream_type { in, out, err }; class redirect_to { public: redirect_to(stream_type to) : to_(to) { } // delegate gets reference to context object to access // other stream behaviors pair<handle, handle> operator()(stream_type st, context &ctx) const { if (to_ == out && st == err) { // init_stdout_behavior() calls stdout_behavior() with correct // parameters and caches handles handle child_stdout = ctx.init_stdout_behavior().first; return make_pair(child_stdout, handle()); } else if ... } private: stream_type to_; }; context ctx; ctx.stdout_behavior = pipe(); ctx.stderr_behavior = redirect_to(out); ---------- init_stdout_behavior() would need to make sure that stream behaviors are initialized only once (not that there are multiple pipes created). But then it's difficult again to reuse a context object as it would need to be reset explicitly. But this could be done in create_child() again. - Comments?
What still has to be done?
* The context class still has a default constructor only which initializes the stream behaviors to inherit standard streams. This is a problem for Windows GUI applications as they don't have standard streams (or at least they are closed). While the Named Constructor Idiom has been proposed what exactly should the done? Should the default constructor be private and developers must explicitly use a factory function to create a context object? Shall the factory function initialize all standard streams with the same behavior? Shall we provide a factory function which expects three parameters to initialize the standard streams? What's convenient and flexible and not too confusing?
I expect that if some std stream is closed in the calling process, the default constructor of the context class doesn't throw anything. As I see it, the bp::handle should have a "dont-close" flag and bp::behavior::inherit shouldn't call dup() or DuplicateHandle().
The reason why bp::behavior::inherit calls dup()/DuplicateHandle() is that a parent closes a child's handles (and a child a parent's handles; this is done in create_child()). Otherwise both processes would have a read and write end open when a pipe is used. Without a dup()/DuplicateHandle() a parent would close its own standard streams if it wants the child to inherit them. I think it also makes logical sense to automatically close the streams of the other process? As I also think the default constructor of context shouldn't throw: What if we add another optional parameter to bp::behavior::inherit? A bool variable could be used to indicate whether bp::behavior::inherit should throw if dup()/DuplicateHandle() fails. By default it throws - but it doesn't when used by the default constructor of context? Anyway, enough ideas for now. :) Boris
Boris Schaeling wrote:
On Wed, 08 Sep 2010 03:37:52 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
init_stdout_behavior() would need to make sure that stream behaviors are initialized only once (not that there are multiple pipes created). But then it's difficult again to reuse a context object as it would need to be reset explicitly. But this could be done in create_child() again. - Comments?
[snip]
I expect that if some std stream is closed in the calling process, the default constructor of the context class doesn't throw anything. As I see it, the bp::handle should have a "dont-close" flag and bp::behavior::inherit shouldn't call dup() or DuplicateHandle().
The reason why bp::behavior::inherit calls dup()/DuplicateHandle() is that a parent closes a child's handles (and a child a parent's handles; this is done in create_child()). Otherwise both processes would have a read and write end open when a pipe is used. Without a dup()/DuplicateHandle() a parent would close its own standard streams if it wants the child to inherit them. I think it also makes logical sense to automatically close the streams of the other process?
As I also think the default constructor of context shouldn't throw: What if we add another optional parameter to bp::behavior::inherit? A bool variable could be used to indicate whether bp::behavior::inherit should throw if dup()/DuplicateHandle() fails. By default it throws - but it doesn't when used by the default constructor of context?
Perhaps policies could manage the degrees of freedom in behavior? Doing so would give names to the various behaviors you've been discussing and would make more operations implicit once the desired policy is specified. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Wed, 08 Sep 2010 23:20:01 +0200 "Boris Schaeling" <boris@highscore.de> wrote:
On Wed, 08 Sep 2010 03:37:52 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]
I'd appreciate if you could comment on the changes. Especially have a look at:
* this signature: std::pair<handle, handle> (bool).
It would be better if delegate would accept fileno instead of bool indicating input/output. bp::behavior::inherit seems almost indistinguishable from the redirect_to.
Which fileno should be passed? stdin/stdout/stderr of the parent process?
typedef int fileno;
Maybe we have a similar idea because I also think it would be better to distinguish standard output and standard error. This could be done with an enumeration and could maybe help to create a better redirect_to stream behavior. The problem with redirect_to though is that it depends on another stream behavior. I don't know whether Boost.Process should support dependencies between stream behaviors?
Yes, at least for redirect_to_stdout behavior. It is fairly common use case. The following should solve problems that I know of: typedef std::vector<std::pair<fileno, handle::native_type>> child_streams; typedef boost::function2< std::pair<handle, handle>, fileno, const child_streams& > behavior; std::pair<handle, handle> inherit(fileno fn, const child_streams&) { #ifdef _WIN32 handle child_end(GetStdHandle(DWORD(-10-fn)), dont_close); #else handle child_end(fn, dont_close); #endif handle parent_end; return std::make_pair(child_end, parent_end); } class redirect_to { fileno to_; public: redirect_to(fileno to): to_(to) {} std::pair<handle, handle> operator()(fileno fn, const child_streams& cs) { BOOST_ASSERT(fn > to_); #ifndef _WIN32 handle child_end(posix_dup2(to_, fn)); #else // ... #endif handle parent_end; return std::make_pair(child_end, parent_end); } }; enum std_fileno { stdin, stdout, stderr }; Example of usage: ctx.stderr = redirect_to(stdout);
[snip]
* The context class still has a default constructor only which
initializes the stream behaviors to inherit standard streams. This is a problem for Windows GUI applications as they don't have standard streams (or at least they are closed). While the Named Constructor Idiom has been proposed what exactly should the done? Should the default constructor be private and developers must explicitly use a factory function to create a context object? Shall the factory function initialize all standard streams with the same behavior? Shall we provide a factory function which expects three parameters to initialize the standard streams? What's convenient and flexible and not too confusing?
I expect that if some std stream is closed in the calling process, the default constructor of the context class doesn't throw anything. As I see it, the bp::handle should have a "dont-close" flag and bp::behavior::inherit shouldn't call dup() or DuplicateHandle().
The reason why bp::behavior::inherit calls dup()/DuplicateHandle() is that a parent closes a child's handles (and a child a parent's handles; this is done in create_child()). Otherwise both processes would have a read and write end open when a pipe is used. Without a dup()/DuplicateHandle() a parent would close its own standard streams
It wouldn't if you add "don't-close" flag.
if it wants the child to inherit them. I think it also makes logical sense to automatically close the streams of the other process?
As I also think the default constructor of context shouldn't throw: What if we add another optional parameter to bp::behavior::inherit? A bool variable could be used to indicate whether bp::behavior::inherit should throw if dup()/DuplicateHandle() fails.
No, it shouldn't call dup()/DuplicateHandle() in the first place.
By default it throws - but it doesn't when used by the default constructor of context?
Anyway, enough ideas for now. :)
I still have some )
On Fri, 10 Sep 2010 03:33:53 +0400 Ilya Sokolov <ilyasokol@gmail.com> wrote:
[snip]
class redirect_to { fileno to_; public:
redirect_to(fileno to): to_(to) {} std::pair<handle, handle> operator()(fileno fn, const child_streams& cs) { BOOST_ASSERT(fn > to_); #ifndef _WIN32 handle child_end(posix_dup2(to_, fn)); #else // ... #endif handle parent_end; return std::make_pair(child_end, parent_end); } };
Not sure who wrote that code ;-) I should stop posting at night. class redirect_to { fileno to_; public: redirect_to(fileno to): to_(to) {} std::pair<handle, handle> operator()(fileno fn, const child_streams& cs) { BOOST_ASSERT(fn > to_); child_streams::const_iterator it = // find pair in cs where it->first == to_ #ifndef _WIN32 handle child_end(posix_dup2(it->second, fn)); #else // ... #endif handle parent_end; return std::make_pair(child_end, parent_end); } };
On Fri, 10 Sep 2010 01:33:53 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]The following should solve problems that I know of:
typedef std::vector<std::pair<fileno, handle::native_type>> child_streams;
typedef boost::function2< std::pair<handle, handle>, fileno, const child_streams& > behavior;
std::pair<handle, handle> inherit(fileno fn, const child_streams&) { #ifdef _WIN32 handle child_end(GetStdHandle(DWORD(-10-fn)), dont_close); #else handle child_end(fn, dont_close); #endif handle parent_end; return std::make_pair(child_end, parent_end); }
class redirect_to { fileno to_; public: redirect_to(fileno to): to_(to) {} std::pair<handle, handle> operator()(fileno fn, const child_streams& cs) { BOOST_ASSERT(fn > to_); child_streams::const_iterator it = // find pair in cs where it->first == to_ #ifndef _WIN32 handle child_end(posix_dup2(it->second, fn)); #else // ... #endif handle parent_end; return std::make_pair(child_end, parent_end); } }; enum std_fileno { stdin, stdout, stderr };
Example of usage:
ctx.stderr = redirect_to(stdout);
I'm a bit confused: If you use fileno like above on Windows (0=stdin, 1=stdout and 2=stderr) then we could also use an enumeration? At least your implementation of inherit makes me think so that it's not the file descriptor itself you are interested in but only the information whether the standard input, output or error stream is configured. Then there would be no difference between your and mine solution? If you are interested in the actual value of fileno though (after all the POSIX implementation of inherit uses fileno directly) I see a couple of problems. Passing fileno to a stream behavior's operator() is actually the opposite of what we expect a stream behavior to do: We want it to create the child's streams for us. For example bp::behavior::pipe should return the child's end of the pipe. That said it would be strange to pass a fileno to bp::behavior::pipe if we actually expect bp::behavior::pipe to return a fileno to us. A more serious problem is that on POSIX a child process can inherit streams with filenos greater than 2 (eg. see <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/file_descriptors_setup.cpp>). If you use bp::behavior::pipe like in the example what fileno should we pass to operator()? The pipe needs to know whether it is used to configure an input or output stream. As on Windows handles are used to represent standard streams what should the first parameter passed to operator() look like? We know that the fileno of the standard error stream of the child process will be 2 on POSIX. But what will the handle look like on Windows? How will redirect_to know it should redirect the standard error stream? Well, fortunately redirect_to doesn't need to access the child's standard error stream on Windows but can simply dup and return the to_ stream. :-) But what kind of information would a handle passed to operator() provide? Thinking it through I'm more convinced that we need to pass the information to a stream behavior whether it's used to configure the standard input, output or error stream. As filenos greater than 2 can be inherited on POSIX a bit field might be better as it would allow us to distinguish input and output streams (a filenos greater than 2 are either input or output streams but no standard streams).
[...]
As I also think the default constructor of context shouldn't throw: What if we add another optional parameter to bp::behavior::inherit? A bool variable could be used to indicate whether bp::behavior::inherit should throw if dup()/DuplicateHandle() fails.
No, it shouldn't call dup()/DuplicateHandle() in the first place.
Convinced - the dont_close flag sounds like a good idea! I'll update the code on the weekend. Boris
On 09/10/2010 03:21 PM, Boris Schaeling wrote:
[snip]
I'm a bit confused: If you use fileno like above on Windows (0=stdin, 1=stdout and 2=stderr) then we could also use an enumeration? At least your implementation of inherit makes me think so that it's not the file descriptor itself you are interested in but only the information whether the standard input, output or error stream is configured. Then there would be no difference between your and mine solution?
It seems that you should just use a different name for the stream behavior depending on which direction of pipe it will create. It would be possible for the user to configure pipes for standard streams in the direction opposite from what is standard (and also perhaps opposite from what the user intended), but such a problem would likely be fairly obvious to figure out. You could also provide a bidirectional pipe (created using socketpair on POSIX, maybe something else on Windows), though it may be better to use pipe on POSIX as people probably expect a unidrectional pipe normally (it seems possible that socketpair uses twice as much kernel buffer space, also). Note: As I mentioned in another message, I think that the parent-end of the pipe should be returned to the user as part of configuring the stream behavior. This would further reduce confusion and chance of incorrectly configuring the direction. The documentation could also address this.
On Sat, 11 Sep 2010 00:45:08 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
On 09/10/2010 03:21 PM, Boris Schaeling wrote:
[snip]
I'm a bit confused: If you use fileno like above on Windows (0=stdin, 1=stdout and 2=stderr) then we could also use an enumeration? At least your implementation of inherit makes me think so that it's not the file descriptor itself you are interested in but only the information whether the standard input, output or error stream is configured. Then there would be no difference between your and mine solution?
It seems that you should just use a different name for the stream behavior depending on which direction of pipe it will create. It would
This is what we basically had before when you had to pass a parameter like input_stream or output_stream: ctx.stdin_behavior = bp::behavior::pipe(input_stream); ctx.stdout_behavior = bp::behavior::pipe(output_stream); Whether you pass a parameter or create two pipe behaviors for input and output streams doesn't make a big difference in the end. Passing the information whether an input or output stream is configured to the delegate automatically will help to avoid mistakes though as you can write: ctx.stdin_behavior = bp::behavior::pipe(); ctx.stdout_behavior = bp::behavior::pipe();
be possible for the user to configure pipes for standard streams in the direction opposite from what is standard (and also perhaps opposite from what the user intended), but such a problem would likely be fairly obvious to figure out. You could also provide a bidirectional pipe
While it's fairly obvious I agree with Ilya (I think he pointed it out) that passing the information to a stream behavior explicitly whether an input or output stream is configured is redundant as stream behaviors are attached to input or output stream delegates anyway.
[...]Note: As I mentioned in another message, I think that the parent-end of the pipe should be returned to the user as part of configuring the stream behavior. This would further reduce confusion and chance of incorrectly configuring the direction. The documentation could also address this.
With pipe you mean the pair of handles created by pipe? Where and why do you want to return it to the user? You want the user to explicitly set the pair of handles in context? Boris
On 09/11/2010 09:07 AM, Boris Schaeling wrote:
[snip]
While it's fairly obvious I agree with Ilya (I think he pointed it out) that passing the information to a stream behavior explicitly whether an input or output stream is configured is redundant as stream behaviors are attached to input or output stream delegates anyway.
The main issue is that I would like the same facilities for configuring std{in,out,err} to also be usable for arbitrary file descriptor numbers on POSIX, and for those file descriptors, you cannot assume any particular direction.
[...]Note: As I mentioned in another message, I think that the parent-end of the pipe should be returned to the user as part of configuring the stream behavior. This would further reduce confusion and chance of incorrectly configuring the direction. The documentation could also address this.
With pipe you mean the pair of handles created by pipe? Where and why do you want to return it to the user? You want the user to explicitly set the pair of handles in context?
I think I explained this more clearly in another e-mail in reply to you.
On Sat, 11 Sep 2010 20:36:17 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]
While it's fairly obvious I agree with Ilya (I think he pointed it out) that passing the information to a stream behavior explicitly whether an input or output stream is configured is redundant as stream behaviors are attached to input or output stream delegates anyway.
The main issue is that I would like the same facilities for configuring std{in,out,err} to also be usable for arbitrary file descriptor numbers on POSIX, and for those file descriptors, you cannot assume any particular direction.
Indeed. But as context doesn't support arbitrary file descriptors those must be configured differently anyway. Please have a look at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/file_descriptors_setup.cpp>. There are two pipes used to create file descriptors 3 and 4 which should be inherited by the child process. As the pipe stream behavior is used explicitly the user knows if 3 and 4 should be read or write ends of the pipe. The user can pass the information to the pipe stream behavior while the context indeed can't (in this example there is still a boolean value used).
[...]Note: As I mentioned in another message, I think that the parent-end of the pipe should be returned to the user as part of configuring the stream behavior. This would further reduce confusion and chance of incorrectly configuring the direction. The documentation could also address this.
With pipe you mean the pair of handles created by pipe? Where and why do you want to return it to the user? You want the user to explicitly set the pair of handles in context?
I think I explained this more clearly in another e-mail in reply to you.
I'll try to find it although I slowly get lost in this huge thread. :) Boris
On 09/11/2010 01:32 PM, Boris Schaeling wrote:
Indeed. But as context doesn't support arbitrary file descriptors those must be configured differently anyway.
I'm suggesting that it should. From the library's perspective, there is no difference between configuring the child's file descriptors 0, 1, and 2, and configuring any other file descriptors, at least on POSIX, and given that it is useful for the user, it might as well be exposed. (Of course, on Windows, things are different, but I think you have just as elegant an interface and still allow this flexibility for POSIX while retaining portable syntax.)
Please have a look at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/file_descriptors_setup.cpp>. There are two pipes used to create file descriptors 3 and 4 which should be inherited by the child process. As the pipe stream behavior is used explicitly the user knows if 3 and 4 should be read or write ends of the pipe. The user can pass the information to the pipe stream behavior while the context indeed can't (in this example there is still a boolean value used).
I saw that example, but it hardly seemed like a very good exposition of the convenience provided by the library. (Also, as I alluded to in some of my other example code snippets and as has been discussed a bit by others, I think that using an std::pair as the return value for those type of things is a bad idea. Instead, read_end and write_end, or parent_end and child_end, depending on the situation, should be used.)
[snip]
On 09/11/2010 01:51 PM, Jeremy Maitin-Shepard wrote:
On 09/11/2010 01:32 PM, Boris Schaeling wrote: [snip]
Please have a look at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/file_descriptors_setup.cpp>.
There are two pipes used to create file descriptors 3 and 4 which should be inherited by the child process. As the pipe stream behavior is used explicitly the user knows if 3 and 4 should be read or write ends of the pipe. The user can pass the information to the pipe stream behavior while the context indeed can't (in this example there is still a boolean value used).
I saw that example, but it hardly seemed like a very good exposition of the convenience provided by the library. (Also, as I alluded to in some of my other example code snippets and as has been discussed a bit by others, I think that using an std::pair as the return value for those type of things is a bad idea. Instead, read_end and write_end, or parent_end and child_end, depending on the situation, should be used.)
By the way, the example is quite possibly not even correct, because file descriptors 3 and 4 might already be in use, and therefore depending on the order of operations (and the relative order of context::setup and boost process's internal file descriptor setup) the code may do the wrong thing. That is why all of the file descriptor setup has to be coordinated, and therefore why boost process needs to provide an interface for doing it.
On Sat, 11 Sep 2010 23:24:32 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
On 09/11/2010 01:51 PM, Jeremy Maitin-Shepard wrote:
On 09/11/2010 01:32 PM, Boris Schaeling wrote: [snip]
Please have a look at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/file_descriptors_setup.cpp>.
[...]By the way, the example is quite possibly not even correct, because file descriptors 3 and 4 might already be in use, and therefore depending on the order of operations (and the relative order of context::setup and boost process's internal file descriptor setup) the code may do the
You are right. If I remember correctly the example isn't tested yet (it was converted from an example from a previous Boost.Process version).
wrong thing. That is why all of the file descriptor setup has to be coordinated, and therefore why boost process needs to provide an interface for doing it.
The interface is context::setup(). The method is called after fork() and before execve() in the child process only (on POSIX). When context::setup() is called (only) the standard streams have been configured and you can do whatever you like with file descriptors inherited from the parent process. This has also been tested in <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/test/child.cpp> (search for "BOOST_AUTO_TEST_CASE(test_posix)"). If you think this test case misses something I'd be happy to add another one. Boris
On 09/11/2010 03:15 PM, Boris Schaeling wrote:
On Sat, 11 Sep 2010 23:24:32 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
On 09/11/2010 01:51 PM, Jeremy Maitin-Shepard wrote:
On 09/11/2010 01:32 PM, Boris Schaeling wrote: [snip]
Please have a look at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/file_descriptors_setup.cpp>.
[...]By the way, the example is quite possibly not even correct, because file descriptors 3 and 4 might already be in use, and therefore depending on the order of operations (and the relative order of context::setup and boost process's internal file descriptor setup) the code may do the
You are right. If I remember correctly the example isn't tested yet (it was converted from an example from a previous Boost.Process version).
wrong thing. That is why all of the file descriptor setup has to be coordinated, and therefore why boost process needs to provide an interface for doing it.
The interface is context::setup(). The method is called after fork() and before execve() in the child process only (on POSIX). When context::setup() is called (only) the standard streams have been configured and you can do whatever you like with file descriptors inherited from the parent process.
The fact that the example is wrong is precisely why this interface isn't sufficient. To fix it, you'd have to do some careful dup gymnastics, all the while checking the return codes of all of the API calls and signaling an error to the parent process somehow. (Not only do you have to check the return codes, you also have to loop and retry if you receive EINTR.) Not having to bother with all of these annoyances is a primary motivation for using a library like Boost.Process in the first place. Suppose you want to map the child's file descriptor 3 to what the parent's file descriptor 1 is, and you also want the child's file descriptor 1 to be set to something else. You configure the child's file descriptor 1 using the library's interface. Since context::setup is called after the child's file descriptor 1 has been remapped, you no longer have access to the parent's file descriptor 1, unless you took care to dup it previously (and ensure that you are returned something > 2). The library has to handle of this complexity for the standard streams, so I'd like it to deal with it for any other file descriptors also.
On Sun, 12 Sep 2010 01:03:59 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]Suppose you want to map the child's file descriptor 3 to what the parent's file descriptor 1 is, and you also want the child's file descriptor 1 to be set to something else. You configure the child's file descriptor 1 using the library's interface. Since context::setup is called after the child's file descriptor 1 has been remapped, you no longer have access to the parent's file descriptor 1, unless you took care to dup it previously (and ensure that you are returned something > 2).
Yes, this is what create_child() does with the standard streams. And I agree you probably have to do the same (calling fcntl(..., F_DUPFD, 3)). But I'm not sure if this POSIX-only problem which can be rather easily solved by the user and is only an issue when a fd > 2 should be mapped to a fd < 3 justifies to change the interface of context (at least I don't expect it will become easier to use)? I would agree that this would definitely need to be documented. But I'm still not sure whether Boost.Process should provide a posix_context class which supports the scenario above out of the box (I can already hear users asking "if the library supports X on POSIX why not Y?"). If we could somehow draw a line between "good" and "bad" platform-specific features it will be much easier for maintainers later. Maybe a direct question helps: Why should Boost.Process support fds > 2 out of the box but not chroot for example (there was a string member variable in context in previous versions to easily set the root directory)? Or your preference is just the opposite and you like to see additional platform-specific classes which support as many platform-specific features as possible? Boris
On 09/12/2010 08:45 AM, Boris Schaeling wrote:
On Sun, 12 Sep 2010 01:03:59 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]Suppose you want to map the child's file descriptor 3 to what the parent's file descriptor 1 is, and you also want the child's file descriptor 1 to be set to something else. You configure the child's file descriptor 1 using the library's interface. Since context::setup is called after the child's file descriptor 1 has been remapped, you no longer have access to the parent's file descriptor 1, unless you took care to dup it previously (and ensure that you are returned something > 2).
Yes, this is what create_child() does with the standard streams. And I agree you probably have to do the same (calling fcntl(..., F_DUPFD, 3)). But I'm not sure if this POSIX-only problem which can be rather easily solved by the user and is only an issue when a fd > 2 should be mapped to a fd < 3 justifies to change the interface of context (at least I don't expect it will become easier to use)? I would agree that this would definitely need to be documented.
Mapping an fd > 2 to an existing fd <= 2 was an extreme example where the Boost Process code was actively interfering, but in fact the naive approach of dup2 followed by close will get you in trouble quite easily. (Suppose you want to assign to both 3 and 4, and it happens that the existing fds are assigned to 4 and 3 respectively, or you want to assign something to e.g. 3 and it happens to already be 3, so you end up closing it, etc.) From my perspective, this means there is a clear need (by any users that need to do mappings > 3) for a library to handle the file descriptor mapping. You make the argument that boost process is not the right library, but if Boost.Process is being used, it seems clear that the functionality could only be provided by Boost.Process. (To have a convenient interface, I think it would clearly need to be coupled with Boost.Process.)
But I'm still not sure whether Boost.Process should provide a posix_context class which supports the scenario above out of the box (I can already hear users asking "if the library supports X on POSIX why not Y?"). If we could somehow draw a line between "good" and "bad" platform-specific features it will be much easier for maintainers later. Maybe a direct question helps: Why should Boost.Process support fds > 2 out of the box but not chroot for example (there was a string member variable in context in previous versions to easily set the root directory)? Or your preference is just the opposite and you like to see additional platform-specific classes which support as many platform-specific features as possible?
I certainly don't want to see Boost.Process providing a way to do arbitrary POSIX things like setuid, etc. A key distinction between setuid, chroot, etc. is that they require only a single system call, so that invoking those functions directly from the callback is practically the optimal interface anyway. I would like to see a more convenient C++ interface to POSIX calls generally, but that can all be done completely independently from Boost.Process (and then potentially used from the callback function). With file descriptor redirection, I think catering more towards POSIX makes sense, because I think it is much more likely to be used on POSIX than on Windows. (My impression is that std{in,out,err} are not used very extensively on Windows at all, and doing any sort of fancy redirection is even rarer.) In general, I think it is good to keep libraries, interfaces, etc. as simple and decoupled as possible, provided that you don't sacrifice power or efficiency in the interface. If you want Boost.Process to be the _the_ c++ interface for child process creation (and really, that is what "boost." implies), then it is important to ensure that all but the most extreme cases can be handled in a useful way (e.g. either the library directly provides a convenient way to do what you want, or the library can be conveniently used in combination with some other library to do what you want), as opposed to having to bypass the library or sacrifice convenience to work with the library.
On Mon, 13 Sep 2010 01:11:33 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]closing it, etc.) From my perspective, this means there is a clear need (by any users that need to do mappings > 3) for a library to handle the file descriptor mapping. You make the argument that boost process is not the right library, but if Boost.Process is being used, it seems clear that the functionality could only be provided by Boost.Process. (To have a convenient interface, I think it would clearly need to be coupled with Boost.Process.)
OK, let's see where we get. Can you write a small sample program (doesn't need to be complete) which shows how you would prefer to configure file descriptors greater than 3? Maybe we can come up with an extension which doesn't make the current API more complicated but still gives you the flexibility you need. Boris
[...]
On 09/13/2010 12:56 PM, Boris Schaeling wrote:
On Mon, 13 Sep 2010 01:11:33 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]closing it, etc.) From my perspective, this means there is a clear need (by any users that need to do mappings > 3) for a library to handle the file descriptor mapping. You make the argument that boost process is not the right library, but if Boost.Process is being used, it seems clear that the functionality could only be provided by Boost.Process. (To have a convenient interface, I think it would clearly need to be coupled with Boost.Process.)
OK, let's see where we get. Can you write a small sample program (doesn't need to be complete) which shows how you would prefer to configure file descriptors greater than 3? Maybe we can come up with an extension which doesn't make the current API more complicated but still gives you the flexibility you need.
To represent existing/open file descriptors, potentially the library could rely on boost::iostreams::file_descriptor{,_sink,_source}. (These classes seem to basically do exactly what is needed and nothing more. They do rely on shared_ptr for reference counting, but reference counting is convenient.) The fundamental operations are: - specifying the default behavior for file descriptors not explicitly configured (inherit or close) - for any particular child file number, specify one of: 1. an existing parent file descriptor to which it should be mapped; 2. that it should be inherited (a special case of 1, in some sense); 3. that it should be closed. To restrict at compile time file numbers to the permitted ones on Windows, the file number could be specified using an enum type with values boost::process::std{in,out,err}, as discussed. Syntax for these basic operations might be: context ctx; ctx[bp::stdout] = some_fd; // Note: some_fd might be an open (filesystem) file, an open socket, etc. inherit(ctx[3]); close(ctx[4]); inherit_by_default(ctx); close_by_default(ctx); On top of these basic operations, facilities for creating pipes, opening /dev/null, etc. could be provided. You could also define e.g. void inherit_standard_streams(context &ctx) { inherit(ctx[stdin]); inherit(ctx[stdout]); inherit(ctx[stderr]); } For pipes, I would think you would first implement a generic pipe facility, e.g. some function create_pipe() that returns a struct with a file_descriptor_source member and file_descriptor_sink member (maybe called source and sink respectively), or possibly std::pair/boost::tuple could be used, since the types are distinct, thereby avoiding confusion. You then need some facility for assigning one end of the pipe to a file descriptor in context, and returning the other end for use in the parent process. Potentially operator overloading could be used for this purpose, or otherwise some function call. Operator overloading has the advantage of succinctly indicating which direction is being assigned, e.g. file_descriptor_sink stdin_sink = ctx[bp::stdin] << create_pipe(); Although this syntax may seem a bit fancy, it would actually all involve very little code, I think. To support /dev/null, you could have define the function: file_descriptor open_null_device(); and then do e.g. ctx[bp::stdin] = open_null_device(); I don't know how convenientlyboost::iostreams integrates with asio, but it seems that is fairly useful independent of Boost Process.
On Tue, 14 Sep 2010 23:15:25 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]Syntax for these basic operations might be:
context ctx;
ctx[bp::stdout] = some_fd; // Note: some_fd might be an open (filesystem) file, an open socket, etc.
This is a syntax I basically like. I'm not sure though whether it should be ctx.stream_behaviors[] instead of ctx[]? As for Windows users this interface shouldn't be more confusing than the current one I prefer it to the hardcoded member variables stdin_behavior, stdout_behavior and stderr_behavior.
[...]Although this syntax may seem a bit fancy, it would actually all involve very little code, I think.
I think for now I'll update the context class to support the syntax above but with existing stream behaviors. This should be a rather simple change hopefully everyone agrees with. Your ideas as interesting as they are open the box of Pandora and will start another huge discussion which I would prefer to engage in after we have managed to finish version 1.0 of Boost.Process (which is hopefully this year ;). Boris
On 09/15/2010 03:09 PM, Boris Schaeling wrote:
On Tue, 14 Sep 2010 23:15:25 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]Syntax for these basic operations might be:
context ctx;
ctx[bp::stdout] = some_fd; // Note: some_fd might be an open (filesystem) file, an open socket, etc.
This is a syntax I basically like. I'm not sure though whether it should be ctx.stream_behaviors[] instead of ctx[]? As for Windows users this interface shouldn't be more confusing than the current one I prefer it to the hardcoded member variables stdin_behavior, stdout_behavior and stderr_behavior.
Potentially .streams to be shorter, and also due to my note below.
[...]Although this syntax may seem a bit fancy, it would actually all involve very little code, I think.
I think for now I'll update the context class to support the syntax above but with existing stream behaviors. This should be a rather simple change hopefully everyone agrees with. Your ideas as interesting as they are open the box of Pandora and will start another huge discussion which I would prefer to engage in after we have managed to finish version 1.0 of Boost.Process (which is hopefully this year ;).
There is likely to be one complication, which is that the .std{in,out,err} members of child_process would not be compatible with the new syntax, both because they don't account for additional file descriptors (could be fixed by using a [fd] interface as well) but also because in the existing interface the stdin member has a different type from the std{out,err} members, which is more problematic. There are certainly various solutions, but I think the cleanest one is to remove those members and instead have the user obtain the parent end of the pipe in the process of configuring the stream. (I imagine in addition to being the cleanest interface in terms of allowing the most to be enforced at compile time, it would also be the simplest to implement.) I imagine you weren't intending to have to change that part of the interface in order to make the change for setting the stream behaviors, but I think it is relatively simple and an improvement in any case.
On Sat, 11 Sep 2010 22:51:25 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
On 09/11/2010 01:32 PM, Boris Schaeling wrote:
Indeed. But as context doesn't support arbitrary file descriptors those must be configured differently anyway.
I'm suggesting that it should. From the library's perspective, there is no difference between configuring the child's file descriptors 0, 1, and 2, and configuring any other file descriptors, at least on POSIX, and given that it is useful for the user, it might as well be exposed. (Of course, on Windows, things are different, but I think you have just as elegant an interface and still allow this flexibility for POSIX while retaining portable syntax.)
I'm not really convinced of adding platform-specific features. We had POSIX- and Windows-only classes and functions in previous versions of the library (in fact that's how my Boost.Process version 0.3 looked like). The implementation of the new version is by far less bloated and much cleaner. But maybe we can add another extension point? In fact it's not required to subclass context as create_child() is a function template. You can create another context class with new member variables and pass it to create_child(). The question is then what exactly would the extension point need to look like and how would it differ from context::setup() which is already provided today?
Please have a look at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/file_descriptors_setup.cpp>. There are two pipes used to create file descriptors 3 and 4 which should be inherited by the child process. As the pipe stream behavior is used explicitly the user knows if 3 and 4 should be read or write ends of the pipe. The user can pass the information to the pipe stream behavior while the context indeed can't (in this example there is still a boolean value used).
I saw that example, but it hardly seemed like a very good exposition of the convenience provided by the library. (Also, as I alluded to in some
I agree. It's a bit strange to use stream behaviors like this. But then stream behaviors were really invented to create a platform-independent way of configuring standard streams and not to support platform-specific features.
of my other example code snippets and as has been discussed a bit by others, I think that using an std::pair as the return value for those type of things is a bad idea. Instead, read_end and write_end, or parent_end and child_end, depending on the situation, should be used.)
We need both ends in create_child() as the function returns a child object. It is the one used by the parent process to possibly communicate with the child process: One end is passed to the child process and the other end remains in the parent process so it can communicate with the child process. Boris
On 09/11/2010 02:47 PM, Boris Schaeling wrote:
On Sat, 11 Sep 2010 22:51:25 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
On 09/11/2010 01:32 PM, Boris Schaeling wrote:
Indeed. But as context doesn't support arbitrary file descriptors those must be configured differently anyway.
I'm suggesting that it should. From the library's perspective, there is no difference between configuring the child's file descriptors 0, 1, and 2, and configuring any other file descriptors, at least on POSIX, and given that it is useful for the user, it might as well be exposed. (Of course, on Windows, things are different, but I think you have just as elegant an interface and still allow this flexibility for POSIX while retaining portable syntax.)
I'm not really convinced of adding platform-specific features. We had POSIX- and Windows-only classes and functions in previous versions of the library (in fact that's how my Boost.Process version 0.3 looked like). The implementation of the new version is by far less bloated and much cleaner.
But maybe we can add another extension point? In fact it's not required to subclass context as create_child() is a function template. You can create another context class with new member variables and pass it to create_child(). The question is then what exactly would the extension point need to look like and how would it differ from context::setup() which is already provided today?
I agree that it is undesirable for the library to have explicit support/a wrapper interface for every single platform-specific feature. For POSIX, the existing function callback interface is exactly the right interface for supporting things like setuid, etc. (As I've mentioned before, though, I'd say the Windows extension interface is a bit lacking, though.) The exception, however, is support for file descriptor mapping, for which the POSIX extension interface is not sufficient, and which is obviously extremely closely related to handling of the "standard streams". In particular, I'd say that with the current interface, anyone that needs to do file descriptor mappings for file numbers > 2 basically shouldn't use boost.process.
Please have a look at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/example/file_descriptors_setup.cpp>.
There are two pipes used to create file descriptors 3 and 4 which should be inherited by the child process. As the pipe stream behavior is used explicitly the user knows if 3 and 4 should be read or write ends of the pipe. The user can pass the information to the pipe stream behavior while the context indeed can't (in this example there is still a boolean value used).
I saw that example, but it hardly seemed like a very good exposition of the convenience provided by the library. (Also, as I alluded to in some
I agree. It's a bit strange to use stream behaviors like this. But then stream behaviors were really invented to create a platform-independent way of configuring standard streams and not to support platform-specific features.
of my other example code snippets and as has been discussed a bit by others, I think that using an std::pair as the return value for those type of things is a bad idea. Instead, read_end and write_end, or parent_end and child_end, depending on the situation, should be used.)
We need both ends in create_child() as the function returns a child object. It is the one used by the parent process to possibly communicate with the child process: One end is passed to the child process and the other end remains in the parent process so it can communicate with the child process.
I was just suggesting the use of a custom struct rather than std::pair, so that you would have named members rather than first and second, to avoid confusion about the meaning of first and second. However, in some other e-mails, I suggest that boost process shouldn't itself hold on to the parent end of pipes and store them in child_process instances for the purpose of providing get_stdin(), etc. Instead, the parent end of the pipe is returned to the caller in the course of setting up the mapping, and then boost process doesn't need to do anything with it after that. I think this leads to a cleaner interface, since with the current interface, get_stdin, etc. are always present in child_process but only meaningful if you happened to have configured a pipe on the corresponding file number. Also, the current interface seems to preclude setting up a pipe between two child processes. Consider the following syntax: context ls_ctx, less_ctx; pipe(ls_ctx[bp::stdout], less_ctx[bp::stdin]); inherit(less_ctx[bp::stdout]); inherit(less_ctx[bp::stderr]); Note: bp::std{in,out,err} would be enum values, and operator[] would have an argument of enum type on Windows, and type int on POSIX, as suggested by Ilya. operator[] would return an object, e.g. fd_map_t that stores the argument and a reference to the context, and would support operator= to assign a mapping (pipe, inherit, etc. would internally invoke this operator=). So you could also do: file_descriptor_t ls_3 = output_pipe(ls_ctx[3]); // ls_ctx[3] is assigned to write end, ls_3 refers to read end. Potentially you could add some safety and/or syntactic convenience by having types like readable_file_descriptor_t, writable_file_descriptor_t, readwrite_file_descriptor_t in addition to file_descriptor_t, with the obvious implicit move conversions allowed, and explicit move conversions allowed between any of them. Then, you could have bp::pipe() return a struct e.g. pipe_t that contains both ends, and define various operators like: write_file_descriptor_t operator>>(pipe_t &&, fd_map_t); write_file_descriptor_t operator<<(fd_map_t, pipe_t &&); read_file_descriptor_t operator>>(fd_map_t, pipe_t &&); read_file_descriptor_t operator<<(pipe_t &&, fd_map_t); void operator>>(fd_map_t, write_file_descriptor_t &&); void operator<<(fd_map_t, read_file_descriptor_t &&); Then you could have: ls_ctx[bp::stdout] >> bp::pipe() >> less_ctx[bp::stdin]; write_file_descriptor_t ls_3 = ls_ctx[bp::stdout] >> bp::pipe(); The additional advantage of tagging the file descriptor types with read/write is that it can provide additional type safety if you then use them with either boost iostreams or boost asio. Note: My imagination might be running a bit too wild with these last syntactic suggestions, but in any case I think it demonstrates that you can support arbitrary file descriptor mappings on POSIX without sacrificing the quality of the portable interface. If you wanted to, you could try to prevent the user from assigning the wrong direction stream to std{out,in,err} using typing (e.g. bp::stdout,bp::stderr would have a type different from that of bp::stdin, and there would be a total of 3 versions of operator[] on POSIX, and two versions on Windows), but I don't think it is really necessary.
On Sun, 12 Sep 2010 00:53:17 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
[...]
We need both ends in create_child() as the function returns a child object. It is the one used by the parent process to possibly communicate with the child process: One end is passed to the child process and the other end remains in the parent process so it can communicate with the child process.
I was just suggesting the use of a custom struct rather than std::pair, so that you would have named members rather than first and second, to avoid confusion about the meaning of first and second.
OK, I'll change it.
However, in some other e-mails, I suggest that boost process shouldn't itself hold on to the parent end of pipes and store them in child_process instances for the purpose of providing get_stdin(), etc. Instead, the parent end of the pipe is returned to the caller in the course of setting up the mapping, and then boost process doesn't need to do anything with it after that. I think this leads to a cleaner interface, since with the current interface, get_stdin, etc. are always present in child_process but only meaningful if you happened to have configured a pipe on the corresponding file number.
I see. As the child object is returned by create_child() though it seems to be a reasonable place to store the parent's ends? Maybe get_stdin() et al shouldn't return a pistream or postream though but the handles? This would also help if you want to use asynchronous I/O only and don't need the stream objects anyway?
[...]Then you could have:
ls_ctx[bp::stdout] >> bp::pipe() >> less_ctx[bp::stdin];
write_file_descriptor_t ls_3 = ls_ctx[bp::stdout] >> bp::pipe();
The additional advantage of tagging the file descriptor types with read/write is that it can provide additional type safety if you then use them with either boost iostreams or boost asio.
Note: My imagination might be running a bit too wild with these last syntactic suggestions, but in any case I think it demonstrates that you can support arbitrary file descriptor mappings on POSIX without sacrificing the quality of the portable interface.
While an interesting idea I think this is better done in a Boost.Pipe library if we don't want Boost.Process to be delayed even further. :) Boris
[...]
On Sat, 11 Sep 2010 00:21:48 +0200 "Boris Schaeling" <boris@highscore.de> wrote:
On Fri, 10 Sep 2010 01:33:53 +0200, Ilya Sokolov <ilyasokol@gmail.com> wrote:
[...]The following should solve problems that I know of:
typedef std::vector<std::pair<fileno, handle::native_type>> child_streams;
typedef boost::function2< std::pair<handle, handle>, fileno, const child_streams& > behavior;
std::pair<handle, handle> inherit(fileno fn, const child_streams&) { #ifdef _WIN32 handle child_end(GetStdHandle(DWORD(-10-fn)), dont_close); #else handle child_end(fn, dont_close); #endif handle parent_end; return std::make_pair(child_end, parent_end); }
class redirect_to { fileno to_; public: redirect_to(fileno to): to_(to) {} std::pair<handle, handle> operator()(fileno fn, const child_streams& cs) { BOOST_ASSERT(fn > to_); child_streams::const_iterator it = // find pair in cs where it->first == to_ #ifndef _WIN32 handle child_end(posix_dup2(it->second, fn)); #else // ... #endif handle parent_end; return std::make_pair(child_end, parent_end); } }; enum std_fileno { stdin, stdout, stderr };
Example of usage:
ctx.stderr = redirect_to(stdout);
I'm a bit confused: If you use fileno like above on Windows (0=stdin, 1=stdout and 2=stderr) then we could also use an enumeration?
I was thinking about the ability to set behavior of fileno > 2 on posix.
At least your implementation of inherit makes me think so that it's not the file descriptor itself you are interested in but only the information whether the standard input, output or error stream is configured.
yes
Then there would be no difference between your and mine solution?
???
[snip]
On Sat, 11 Sep 2010 00:21:48 +0200, Boris Schaeling <boris@highscore.de> wrote:
[...]
As I also think the default constructor of context shouldn't throw: What if we add another optional parameter to bp::behavior::inherit? A bool variable could be used to indicate whether bp::behavior::inherit should throw if dup()/DuplicateHandle() fails.
No, it shouldn't call dup()/DuplicateHandle() in the first place.
Convinced - the dont_close flag sounds like a good idea! I'll update the code on the weekend.
Just a quick note that I didn't manage to update anything on the weekend. I made a todo list though so that no ideas get lost. I'll send another mail when I've updated the code. Boris
This is a library of significant interest to me, since particularly under POSIX child process creation and management is rather complicated to do right. Based on looking at the documentation on the website, and browsing through the discussion that has taken place on this list, I have some comments. I understand that the website does not reflect the planned changes to the stream behavior configuration interface. - I suggest some interface changes for the facilities for retrieving the parent side of pipes created for stdin, stdout, and stderr of the child: - The std::istream/std::ostream interface should be decoupled from the facility for retrieving the file descriptor (int) or Windows HANDLE, to avoid the overhead of allocating a streambuf for those users that wish to access the file descriptor/HANDLE directly or use it with asio. Instead, the std::istream/std::ostream interface to a file descriptor/HANDLE could be provided as a separate, completely independent facility (designed so that exactly the same syntax could be used on both Windows and POSIX), and likewise a separate facility for providing an asio interface to a file descriptor/HANDLE (which also allows exactly the same syntax to be used on POSIX and Windows). - It seems it would be cleaner for the parent side of these pipes to be returned in the course of specifying that pipes should be used (in the current terminology, in the course of specifying the pipe stream behavior). This prevents context from being reusable, but it seems that it would be reusable only a very limited set of circumstances anyway. (E.g. if the context::setup method needs additional invocation-specific parameters, etc.) Furthermore, it seems inconsistent that certain things are part of the "reusable" context, like the environment and process_name, but other things, like the executable path and arguments, are not. - On POSIX, due to the complicated semantics of waitpid, etc. and SIGCHLD, and the general desire to avoid leaving around zombie child processes, any code that deals with child processes needs to coordinate with all other code that deals with child processes. The library should explicitly document what it does with respect to SIGCHLD signals and waiting on child processes (and possibly change the behavior if the current behavior doesn't handle all cases properly), so that other code can correctly interact with it. - As was observed in the discussion regarding setting up stdin/stdout/stderr file descriptors on POSIX, it is somewhat tricky to correctly set up child file descriptors. As I believe it is fairly common on POSIX to want to set up child file descriptors other than 0, 1, and 2, it would be best for the library to provide explicit support for this, rather than forcing the user to implement it manually in context::setup. A reasonable interface might be e.g.: context::map_file_descriptor(int child_fileno, file_descriptor_t existing_descriptor); Potentially this same interface could be used on Windows, with child_fileno only valid in the range 0 to 2 (and with appropriate constants defined), and could thus serve as the lowest level interface on top of which other behaviors like pipe, redirect, etc. could be implemented. - On POSIX, the precise stage in the child process setup at which context::setup is called should be documented. - One potential issue is that the Windows extension interface only exposes a (relatively) small subset of the functionality that can be configured on Windows. There seem to be numerous flags to CreateProcess that are not exposed by Boost.Process, and there are also two other functions, CreateProcessAsUser and CreateProcessWithLoginW, which allow specifying some additional options. I am not a Windows programmer, so I don't know how important this additional functionality is in practice, but it seems it would be unfortunate for users that would like to use other conveniences provided by Boost.Process (such as pipe setup, perhaps) to have to revert to invoking the Windows API directly in order to make use of additional functionality not exposed. In contrast, it is hard to imagine the POSIX extension interface being insufficient. I'm not sure how this could be solved, exactly. - I think it would be cleaner for the POSIX and Windows extension interfaces to be in the form of a callback function (templated or boost::function), rather than a subclass of context that overrides the setup function. - It seems that natively on Windows, unlike on POSIX, there is no notion of an array of command line arguments --- there is just a single string command line. It seems it would be useful for Boost Process to allow a single string command line to be specified directly, in addition to the existing array (Container) interface which presumably does some sort of encoding into a single string on Windows. Perhaps create_child can simply distinguish (at compile-time) between being passed a range of char and being passed a range of range of char. - On Windows, it seems there should be both a char and wchar_t version of everything that involves characters. I suggest that the wchar_t version simply not exist on POSIX, as it wouldn't make much sense nor would it be useful.
Jeremy Maitin-Shepard wrote:
- The std::istream/std::ostream interface should be decoupled from the facility for retrieving the file descriptor (int) or Windows HANDLE, to avoid the overhead of allocating a streambuf for those users that wish to access the file descriptor/HANDLE directly or use it with asio. Instead, the std::istream/std::ostream interface to a file descriptor/HANDLE could be provided as a separate, completely independent facility (designed so that exactly the same syntax could be used on both Windows and POSIX), and likewise a separate facility for providing an asio interface to a file descriptor/HANDLE (which also allows exactly the same syntax to be used on POSIX and Windows).
+1 That would be useful in many contexts.
- As was observed in the discussion regarding setting up stdin/stdout/stderr file descriptors on POSIX, it is somewhat tricky to correctly set up child file descriptors. As I believe it is fairly common on POSIX to want to set up child file descriptors other than 0, 1, and 2, it would be best for the library to provide explicit support for this, rather than forcing the user to implement it manually in context::setup. A reasonable interface might be e.g.: context::map_file_descriptor(int child_fileno, file_descriptor_t existing_descriptor);
Potentially this same interface could be used on Windows, with child_fileno only valid in the range 0 to 2 (and with appropriate constants defined), and could thus serve as the lowest level interface on top of which other behaviors like pipe, redirect, etc. could be implemented.
The problem with the suggested approach is that Windows winds up with runtime errors if the child_fileno argument exceeds the range. It would be better to make that a compile time error. The only way to do that is to use a template with the fileno being a template argument. Whether that takes the form of a fileno class template or the fileno is a template argument for map_file_descriptor (which must then be a member function template), is another matter.
- I think it would be cleaner for the POSIX and Windows extension interfaces to be in the form of a callback function (templated or boost::function), rather than a subclass of context that overrides the setup function.
Why do you make that assertion? What becomes cleaner?
- It seems that natively on Windows, unlike on POSIX, there is no notion of an array of command line arguments --- there is just a single string command line. It seems it would be useful for Boost Process to allow a single string command line to be specified directly, in addition to the existing array (Container) interface which presumably does some sort of encoding into a single string on Windows. Perhaps create_child can simply distinguish (at compile-time) between being passed a range of char and being passed a range of range of char.
A general facility for converting between the two representations would be most helpful. For example, one may wish to report the command line, so accessing it as a single string is useful on POSIX and Windows systems. Likewise, one may wish to process the arguments one at a time, as with argc/argv, even on Windows. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On 09/10/2010 04:26 AM, Stewart, Robert wrote:
[snip]
The problem with the suggested approach is that Windows winds up with runtime errors if the child_fileno argument exceeds the range. It would be better to make that a compile time error. The only way to do that is to use a template with the fileno being a template argument. Whether that takes the form of a fileno class template or the fileno is a template argument for map_file_descriptor (which must then be a member function template), is another matter.
For POSIX, it would be unfortunate to require the child fileno be specified as a compile-time constant. However, perhaps the interface can be constructed so that stdin, stdout, stderr are handled at compile-time on Windows (e.g. being distinct types), while allowing the same syntax to result in runtime handling on POSIX.
- I think it would be cleaner for the POSIX and Windows extension interfaces to be in the form of a callback function (templated or boost::function), rather than a subclass of context that overrides the setup function.
Why do you make that assertion? What becomes cleaner?
For one thing, with C++1x lambdas, it would be more convenient to define a function than to define a subclass, particularly if you are making use of variable capture. Likewise, if using boost::bind, boost phoenix, etc. Also, it is unclear if you are supposed to be modifying the environment, stream behaviors, etc. from within the setup function, or whether they have already been processed (I would guess already processed) --- if so, why at such a late stage? --- and if not, why is setup a method of context, if none of the state in context is actually relevant?
- It seems that natively on Windows, unlike on POSIX, there is no notion of an array of command line arguments --- there is just a single string command line. It seems it would be useful for Boost Process to allow a single string command line to be specified directly, in addition to the existing array (Container) interface which presumably does some sort of encoding into a single string on Windows. Perhaps create_child can simply distinguish (at compile-time) between being passed a range of char and being passed a range of range of char.
A general facility for converting between the two representations would be most helpful. For example, one may wish to report the command line, so accessing it as a single string is useful on POSIX and Windows systems. Likewise, one may wish to process the arguments one at a time, as with argc/argv, even on Windows.
Possibly that would be useful, though the encoding of an array of strings into a single string could be somewhat tricky, and would likely need to differ between POSIX and Windows. On both platforms, you would presumably wrap all arguments in double quotes, but the handling of quotes and backslashes would differ: on POSIX, you would presumably use C-style quoting of quote and backslash characters, which is also what most of the shells use. On Windows, it seems there is a different convention, in which backslashes not followed by a quote are not interpreted as escape characters, as described at http://msdn.microsoft.com/en-us/library/17w5ykft.aspx and http://msdn.microsoft.com/en-us/library/ms647232.aspx I'm not sure whether there is any useful convention for non-printable/control characters on either platform --- perhaps best to leave them unchanged, since they are rare anyway. However, given that you have to manually invoke a function like CommandLineToArgvW to obtain the command-line as an array on Windows, I expect that many programs use their own parsing, which may have different behavior.
Jeremy Maitin-Shepard wrote:
On 09/10/2010 04:26 AM, Stewart, Robert wrote:
The problem with the suggested approach is that Windows winds up with runtime errors if the child_fileno argument exceeds the range. It would be better to make that a compile time error. The only way to do that is to use a template with the fileno being a template argument. Whether that takes the form of a fileno class template or the fileno is a template argument for map_file_descriptor (which must then be a member function template), is another matter.
For POSIX, it would be unfortunate to require the child fileno be specified as a compile-time constant. However, perhaps the interface can be constructed so that stdin, stdout, stderr are handled at compile-time on Windows (e.g. being distinct types), while allowing the same syntax to result in runtime handling on POSIX.
I'm not sure if there is a nice solution for both sides of the fence, but I agree that a compile-time constant for POSIX would be a pain for most use cases. Maybe there can be a std_handle class template that is supported on both platforms while the more general, runtime fileno-based variant, is only available on POSIX. Then, developers can choose to use the portable, but restricted std_handle or the non-portable, but unrestricted general type.
- I think it would be cleaner for the POSIX and Windows extension interfaces to be in the form of a callback function (templated or boost::function), rather than a subclass of context that overrides the setup function.
Why do you make that assertion? What becomes cleaner?
For one thing, with C++1x lambdas, it would be more convenient to define a function than to define a subclass, particularly if you are making use of variable capture. Likewise, if using boost::bind, boost phoenix, etc.
Excellent point.
Also, it is unclear if you are supposed to be modifying the environment, stream behaviors, etc. from within the setup function, or whether they have already been processed (I would guess already processed) --- if so, why at such a late stage? --- and if not, why is setup a method of context, if none of the state in context is actually relevant?
That, obviously, could be addressed by documentation, but using the Template Method Pattern would ensure correct sequencing. If that's still insufficient, then it may be that the various configuration steps should be handled in less monolithic classes.
- It seems that natively on Windows, unlike on POSIX, there is no notion of an array of command line arguments --- there is just a single string command line. It seems it would be useful for Boost Process to allow a single string command line to be specified directly, in addition to the existing array (Container) interface which presumably does some sort of encoding into a single string on Windows. Perhaps create_child can simply distinguish (at compile-time) between being passed a range of char and being passed a range of range of char.
A general facility for converting between the two representations would be most helpful. For example, one may wish to report the command line, so accessing it as a single string is useful on POSIX and Windows systems. Likewise, one may wish to process the arguments one at a time, as with argc/argv, even on Windows.
Possibly that would be useful, though the encoding of an array of strings into a single string could be somewhat tricky, and would likely need to differ between POSIX and Windows. On both platforms, you would presumably wrap all arguments in double quotes, but the handling of quotes and backslashes would differ: on POSIX, you would presumably use C-style quoting of quote and backslash characters, which is also what most of the shells use. On Windows, it seems there is a different convention, in which backslashes not followed by a quote are not interpreted as escape characters, as described at
http://msdn.microsoft.com/en-us/library/17w5ykft.aspx
and
http://msdn.microsoft.com/en-us/library/ms647232.aspx
I'm not sure whether there is any useful convention for non-printable/control characters on either platform --- perhaps best to leave them unchanged, since they are rare anyway.
However, given that you have to manually invoke a function like CommandLineToArgvW to obtain the command-line as an array on Windows, I expect that many programs use their own parsing, which may have different behavior.
You raise interesting issues. Windows gives the ability to convert a single string command line to argc/argv using the function you mentioned, so that would, presumably, do the right thing for that platform. Going the other way is less obvious. On POSIX, it is pretty straightforward. One could optimize the output by not quoting arguments without special characters, of course. There's no need for format portability between platforms, just code portability. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Fri, 10 Sep 2010 07:26:06 -0400 "Stewart, Robert" <Robert.Stewart@sig.com> wrote:
Jeremy Maitin-Shepard wrote:
[snip] context::map_file_descriptor(int child_fileno, file_descriptor_t existing_descriptor);
Potentially this same interface could be used on Windows, with child_fileno only valid in the range 0 to 2 (and with appropriate constants defined), and could thus serve as the lowest level interface on top of which other behaviors like pipe, redirect, etc. could be implemented.
The problem with the suggested approach is that Windows winds up with runtime errors if the child_fileno argument exceeds the range. It would be better to make that a compile time error. The only way to do that is to use a template with the fileno being a template argument.
enum std_fileno { stdin, stdout, stderr } #ifdef _WIN32 typedef std_fileno fileno; #else typedef int fileno; #endif void foo(fileno); foo(stdout) // ok foo(1) // ok on posix, compile error on windows
Ilya Sokolov wrote:
"Stewart, Robert" <Robert.Stewart@sig.com> wrote:
The problem with the suggested approach is that Windows winds up with runtime errors if the child_fileno argument exceeds the range. It would be better to make that a compile time error. The only way to do that is to use a template with the fileno being a template argument.
enum std_fileno { stdin, stdout, stderr } #ifdef _WIN32 typedef std_fileno fileno; #else typedef int fileno; #endif
void foo(fileno);
foo(stdout) // ok foo(1) // ok on posix, compile error on windows
That's not bad, but it isn't sufficient. The file number may be generated as a small non-negative integer from various sources, even if it is 0, 1, or 2. The enumerators are initialized with the right values, but other values or integers (int or literals) cannot then be used. A file_number class could be defined that converts from integer and throws an exception on negative or too-large values. 0, 1, and 2 could be guaranteed to not fail at runtime on POSIX and Windows, of course. Static instances for stdin, stdout, and stderr could be provided to avoid the runtime range check, if they prove valuable. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Thu, 09 Sep 2010 20:02:54 -0700 Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
This is a library of significant interest to me, since particularly under POSIX child process creation and management is rather complicated to do right. Based on looking at the documentation on the website, and browsing through the discussion that has taken place on this list, I have some comments. I understand that the website does not reflect the planned changes to the stream behavior configuration interface.
- I suggest some interface changes for the facilities for retrieving the parent side of pipes created for stdin, stdout, and stderr of the child: - The std::istream/std::ostream interface should be decoupled from the facility for retrieving the file descriptor (int) or Windows HANDLE, to avoid the overhead of allocating a streambuf
Could be solved using some form of lazy initialization
for those users that wish to access the file descriptor/HANDLE directly or use it with asio. Instead, the std::istream/std::ostream interface to a file descriptor/HANDLE could be provided as a separate, completely independent facility
Boost.IOStream?
(designed so that exactly the same syntax could be used on both Windows and POSIX), and likewise a separate facility for providing an asio interface to a file descriptor/HANDLE (which also allows exactly the same syntax to be used on POSIX and Windows). - It seems it would be cleaner
Not sure
for the parent side of these pipes to be returned in the course of specifying that pipes should be used (in the current terminology, in the course of specifying the pipe stream behavior).
[snip]
- As was observed in the discussion regarding setting up stdin/stdout/stderr file descriptors on POSIX, it is somewhat tricky to correctly set up child file descriptors. As I believe it is fairly common on POSIX to want to set up child file descriptors other than 0, 1, and 2, it would be best for the library to provide explicit support for this, rather than forcing the user to implement it manually in context::setup. A reasonable interface might be e.g.: context::map_file_descriptor(int child_fileno, file_descriptor_t existing_descriptor);
Not sure what it means (. The most obvious extension to the current interface is: context::set_behavior(fileno fn, function<...> b);
Potentially this same interface could be used on Windows, with child_fileno only valid in the range 0 to 2 (and with appropriate constants defined), and could thus serve as the lowest level interface
Why is it needed? on top of which other behaviors like pipe, redirect, etc.
could be implemented.
- On POSIX, the precise stage in the child process setup at which context::setup is called should be documented.
+1
- One potential issue is that the Windows extension interface only exposes a (relatively) small subset of the functionality that can be configured on Windows. There seem to be numerous flags to CreateProcess that are not exposed by Boost.Process, and there are also two other functions, CreateProcessAsUser and CreateProcessWithLoginW, which allow specifying some additional options.
The interface could be made so complex that winapi would become easier.
I am not a Windows programmer, so I don't know how important this additional functionality is in practice, but it seems it would be unfortunate for users that would like to use other conveniences provided by Boost.Process (such as pipe setup, perhaps) to have to revert to invoking the Windows API directly in order to make use of additional functionality not exposed. In contrast, it is hard to imagine the POSIX extension interface being insufficient. I'm not sure how this could be solved, exactly.
- I think it would be cleaner for the POSIX and Windows extension interfaces to be in the form of a callback function (templated or boost::function), rather than a subclass of context that overrides the setup function.
+1
On 09/10/2010 03:49 PM, Ilya Sokolov wrote:
On Thu, 09 Sep 2010 20:02:54 -0700 Jeremy Maitin-Shepard<jeremy@jeremyms.com> wrote: [snip]
- The std::istream/std::ostream interface should be decoupled from the facility for retrieving the file descriptor (int) or Windows HANDLE, to avoid the overhead of allocating a streambuf
Could be solved using some form of lazy initialization
I suppose, but then you are still unnecessarily coupling a particular buffering and formatting/parsing facility with the management of subprocesses. With lazy initialization, even if you don't use any of the istream/ostream stuff, it still appears to "be there."
for those users that wish to access the file descriptor/HANDLE directly or use it with asio. Instead, the std::istream/std::ostream interface to a file descriptor/HANDLE could be provided as a separate, completely independent facility
Boost.IOStream?
Indeed, so Boost.Process can merely suggest using file_descriptor_source/sink from Boost.IOStream.
[snip]
Not sure what it means (. The most obvious extension to the current interface is:
context::set_behavior(fileno fn, function<...> b);
Yeah, sure. The redirection through a callback function seems unnecessary, though. See my reply to Boris Schaeling.
Potentially this same interface could be used on Windows, with child_fileno only valid in the range 0 to 2 (and with appropriate constants defined), and could thus serve as the lowest level interface
Why is it needed?
The enum approach you described would be better, I think.
[snip]
- One potential issue is that the Windows extension interface only exposes a (relatively) small subset of the functionality that can be configured on Windows. There seem to be numerous flags to CreateProcess that are not exposed by Boost.Process, and there are also two other functions, CreateProcessAsUser and CreateProcessWithLoginW, which allow specifying some additional options.
The interface could be made so complex that winapi would become easier.
I agree that is a concern, but you have to admit, the POSIX extension interface is quite powerful, to the point that there should be little need to bypass boost.process, while the Windows extension interface is not very powerful at all, only providing access to a small subset of the many options available. It is also the case, though, that in many ways process creation through the Windows API is much more straightforward than process creation through the POSIX API, and therefore the abstraction provided by a library like Boost Process is less critical on Windows, particularly for advanced uses. Potentially there could be two different extension interfaces on Windows, with the basic one similar to the existing one except for maybe allowing a few additional flags to be specified, and the advanced one could be fully general by allowing the user to call an alternate API function like CreateProcessWithLoginW, and users that don't need this level of control could simply ignore it.
[snip]
Jeremy Maitin-Shepard wrote:
The POSIX extension interface is quite powerful, to the point that there should be little need to bypass boost.process, while the Windows extension interface is not very powerful at all, only providing access to a small subset of the many options available. It is also the case, though, that in many ways process creation through the Windows API is much more straightforward than process creation through the POSIX API, and therefore the abstraction provided by a library like Boost.Process is less critical on Windows, particularly for advanced uses. Potentially there could be two different extension interfaces on Windows, with the basic one similar to the existing one except for maybe allowing a few additional flags to be specified, and the advanced one could be fully general by allowing the user to call an alternate API function like CreateProcessWithLoginW, and users that don't need this level of control could simply ignore it.
In these particular cases, anyway, the issue is which function to call and which options to provide to create the child process. The function that triggers that behavior could be overloaded such that the caller can supply a callback to invoke that returns a handle to the new process. Boost.Process could supply arguments to the callback to indicate which options must be used when creating the new process and it would be the caller's responsibility to supply a well-behaved callback that creates a process with the attributes required by Boost.Process. Thus, in the normal case, Boost.Process creates the child process in the usual way, with no callback, but in custom use cases, the client supplies a callback to invoke a different API or to specify options Boost.Process wouldn't ordinarily specify. Boost.Process is at the mercy of the caller to do the right thing, but it is clean and does make possible platform-specific customization. Obviously, this same approach is possible for other customization points. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Fri, 10 Sep 2010 05:02:54 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote: Jeremy, I'll go through your feedback quickly as it's pretty late here. :)
[...] - The std::istream/std::ostream interface should be decoupled from the facility for retrieving the file descriptor (int) or Windows HANDLE, to avoid the overhead of allocating a streambuf for those users that wish to access the file descriptor/HANDLE directly or use it with asio. Instead, the std::istream/std::ostream interface to a file descriptor/HANDLE could be provided as a separate, completely independent facility (designed so that exactly the same syntax could be used on both Windows and POSIX), and likewise a separate facility for providing an asio interface to a file descriptor/HANDLE (which also allows exactly the same syntax to be used on POSIX and Windows).
Can you show some sample code you would like to write? Then I think I understand better how Boost.Process would need to be changed.
- It seems it would be cleaner for the parent side of these pipes to be returned in the course of specifying that pipes should be used (in the current terminology, in the course of specifying the pipe stream behavior). This prevents context from being reusable, but it seems that it would be reusable only a very limited set of circumstances anyway. (E.g. if the context::setup method needs additional invocation-specific
Not sure what you mean - can you show some sample code again? :)
parameters, etc.) Furthermore, it seems inconsistent that certain things are part of the "reusable" context, like the environment and process_name, but other things, like the executable path and arguments, are not.
So far reusing a context means launching the very same application a second time. This didn't work first with the stream behavior class hierarchy but works now with the delegates.
- On POSIX, due to the complicated semantics of waitpid, etc. and SIGCHLD, and the general desire to avoid leaving around zombie child processes, any code that deals with child processes needs to coordinate with all other code that deals with child processes. The library should explicitly document what it does with respect to SIGCHLD signals and waiting on child processes (and possibly change the behavior if the current behavior doesn't handle all cases properly), so that other code can correctly interact with it.
I agree that the documentation should be improved. It was created from scratch for Boost.Process 0.4 and doesn't cover yet really everything.
- As was observed in the discussion regarding setting up stdin/stdout/stderr file descriptors on POSIX, it is somewhat tricky to correctly set up child file descriptors. As I believe it is fairly common on POSIX to want to set up child file descriptors other than 0, 1, and 2, it would be best for the library to provide explicit support for this, rather than forcing the user to implement it manually in context::setup. A reasonable interface might be e.g.: context::map_file_descriptor(int child_fileno, file_descriptor_t existing_descriptor);
Once we start to support platform-specific features the question is where do we end? The context class in previous Boost.Process versions grew more and more as support for uid, euid, gid, eguid, chroot etc. was added. Instead of arguing about each and every feature whether it's justified to hardcode it into the library we decided it might be better to support cross-platform features only and provide extension points.
[...]- One potential issue is that the Windows extension interface only exposes a (relatively) small subset of the functionality that can be configured on Windows. There seem to be numerous flags to CreateProcess that are not exposed by Boost.Process, and there are also two other functions, CreateProcessAsUser and CreateProcessWithLoginW, which allow specifying some additional options. I am not a Windows programmer, so I don't know how important this additional functionality is in practice, but it seems it would be unfortunate for users that would like to use other conveniences provided by Boost.Process (such as pipe setup, perhaps) to have to revert to invoking the Windows API directly in order to make use of additional functionality not exposed. In contrast, it is hard to imagine the POSIX extension interface being insufficient. I'm not sure how this could be solved, exactly.
Good point! Indeed a bit more flexibility would be nice here.
- I think it would be cleaner for the POSIX and Windows extension interfaces to be in the form of a callback function (templated or boost::function), rather than a subclass of context that overrides the setup function.
Makes also sense to me!
- It seems that natively on Windows, unlike on POSIX, there is no notion of an array of command line arguments --- there is just a single string command line. It seems it would be useful for Boost Process to allow a single string command line to be specified directly, in addition to the existing array (Container) interface which presumably does some sort of encoding into a single string on Windows. Perhaps create_child can simply distinguish (at compile-time) between being passed a range of char and being passed a range of range of char.
Again sounds good. :)
- On Windows, it seems there should be both a char and wchar_t versionof everything that involves characters. I suggest that the wchar_t version simply not exist on POSIX, as it wouldn't make much sense nor would it be useful.
I don't remember anymore all the details which made it difficult to add support for wide chars in previous Boost.Process versions. But this is something I still want to look into (probably after everything else has been settled). Thanks for your feedback, Boris
On 09/10/2010 03:57 PM, Boris Schaeling wrote:
On Fri, 10 Sep 2010 05:02:54 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
Jeremy,
I'll go through your feedback quickly as it's pretty late here. :)
[...] - The std::istream/std::ostream interface should be decoupled from the facility for retrieving the file descriptor (int) or Windows HANDLE, to avoid the overhead of allocating a streambuf for those users that wish to access the file descriptor/HANDLE directly or use it with asio. Instead, the std::istream/std::ostream interface to a file descriptor/HANDLE could be provided as a separate, completely independent facility (designed so that exactly the same syntax could be used on both Windows and POSIX), and likewise a separate facility for providing an asio interface to a file descriptor/HANDLE (which also allows exactly the same syntax to be used on POSIX and Windows).
Can you show some sample code you would like to write? Then I think I understand better how Boost.Process would need to be changed.
See below.
- It seems it would be cleaner for the parent side of these pipes to be returned in the course of specifying that pipes should be used (in the current terminology, in the course of specifying the pipe stream behavior). This prevents context from being reusable, but it seems that it would be reusable only a very limited set of circumstances anyway. (E.g. if the context::setup method needs additional invocation-specific
Not sure what you mean - can you show some sample code again? :)
file_descriptor_t stdout_pipe = output_pipe(ctx, bp::stdout); file_descriptor_t stdin_pipe = input_pipe(ctx, bp::stdin) (file_descriptor_t would be some type that manages the lifetime of the int/HANDLE.) bp::std{in,out,err} would be enum values, and on posix, you could also use a numeric fd. output_pipe and input_pipe would be convenience functions that couple the creation of a pipe with setting up a file descriptor redirection on ctx, e.g. something like file_descriptor_t output_pipe(context &ctx, fileno fd) { pipe_t p = create_pipe(); ctx.map_fd(fd, boost::move(p.write_end)); return boost::move(p.read_end); } I'm not sure what the status of boost::move is, but... Alternatively, file_descriptor_t could be reference counted, though I like moveable better. You could then use asio, boost iostreams, or something else to interact with these file_descriptors.
parameters, etc.) Furthermore, it seems inconsistent that certain things are part of the "reusable" context, like the environment and process_name, but other things, like the executable path and arguments, are not.
So far reusing a context means launching the very same application a second time. This didn't work first with the stream behavior class hierarchy but works now with the delegates.
It seems that it is questionable how useful this would actually be in most applications, and with the added overhead of redirection through boost::function, it might well actually be more efficient to just obtain the same effect by writing a function that takes care of creating the context and calling create_process, etc.
- On POSIX, due to the complicated semantics of waitpid, etc. and SIGCHLD, and the general desire to avoid leaving around zombie child processes, any code that deals with child processes needs to coordinate with all other code that deals with child processes. The library should explicitly document what it does with respect to SIGCHLD signals and waiting on child processes (and possibly change the behavior if the current behavior doesn't handle all cases properly), so that other code can correctly interact with it.
I agree that the documentation should be improved. It was created from scratch for Boost.Process 0.4 and doesn't cover yet really everything.
- As was observed in the discussion regarding setting up stdin/stdout/stderr file descriptors on POSIX, it is somewhat tricky to correctly set up child file descriptors. As I believe it is fairly common on POSIX to want to set up child file descriptors other than 0, 1, and 2, it would be best for the library to provide explicit support for this, rather than forcing the user to implement it manually in context::setup. A reasonable interface might be e.g.: context::map_file_descriptor(int child_fileno, file_descriptor_t existing_descriptor);
Once we start to support platform-specific features the question is where do we end? The context class in previous Boost.Process versions grew more and more as support for uid, euid, gid, eguid, chroot etc. was added. Instead of arguing about each and every feature whether it's justified to hardcode it into the library we decided it might be better to support cross-platform features only and provide extension points.
I don't suggest that Boost.Process should provide an interface to specific Windows features; however, I believe the current Windows extension interface is not sufficiently powerful, whereas the POSIX one is. However, I think file descriptor setup on POSIX is a special case, because it is something the library is already doing anyway (simply restricted to 0, 1, and 2), and is also something that is fairly complicated to implement correctly, in contrast to e.g. setuid which just involves a single library call.
[snip]
- On Windows, it seems there should be both a char and wchar_t versionof everything that involves characters. I suggest that the wchar_t version simply not exist on POSIX, as it wouldn't make much sense nor would it be useful.
I don't remember anymore all the details which made it difficult to add support for wide chars in previous Boost.Process versions. But this is something I still want to look into (probably after everything else has been settled).
It seems it could either be done with templates, or even just using the preprocessor. Somehow supporting wchar_t on POSIX would naturally be a lot more difficult, but it wouldn't be useful anyway. As an additional comment that I didn't mention before, and seems to be related to some recent comments made in this thread regarding the fact that POSIX can inherit more than just stdin,stdout,stderr, there needs to be a facility on POSIX for specifying the default behavior for file descriptors not otherwise specified, e.g. either inherit or close.
Boris Schaeling wrote:
On Fri, 10 Sep 2010 05:02:54 +0200, Jeremy Maitin-Shepard <jeremy@jeremyms.com> wrote:
Furthermore, it seems inconsistent that certain things are part of the "reusable" context, like the environment and process_name, but other things, like the executable path and arguments, are not.
So far reusing a context means launching the very same application a second time. This didn't work first with the stream behavior class hierarchy but works now with the delegates.
Is there much demand for launching the same process repeatedly? Even if so, that doesn't fit my idea of a "context." To me, "context" would capture things about the current process. What you've called "context" seems much more like "child_process" to me. Have I missed something?
Once we start to support platform-specific features the question is where do we end? The context class in previous Boost.Process versions grew more and more as support for uid, euid, gid, eguid, chroot etc. was added. Instead of arguing about each and every feature whether it's justified to hardcode it into the library we decided it might be better to support cross-platform features only and provide extension points.
The answer to what should be included and not should be based, in part, on how common the need proves to be. That is, rather than be biased against something because it isn't portable, consider whether the library can be truly helpful. Even if the feature is unique to one platform, if the library can offer valuable support for a feature used regularly on that platform, then supporting that feature in a platform-specific namespace would be helpful. Once you allow for platform-specific functionality, there can be demands for a great deal that diverges wildly among the supported platforms and leaves relatively little in the cross-platform portion, so caution is still needed. However, if much of the platform-specific support is merely by providing hooks into the library's behavior, such as the callback I suggested for creating a process, then there shouldn't be too much need for platform-specific functionality in the library. That is, you can allow such functionality but quite possibly preclude the need for it via customization hooks. The customization hook approach also leaves wide open what the library client can do, provided it can be done within the constraints of the interface provided.
- I think it would be cleaner for the POSIX and Windows extension interfaces to be in the form of a callback function (templated or boost::function), rather than a subclass of context that overrides the setup function.
Makes also sense to me!
Perhaps you both were thinking of what I've been suggested herein and in my other reply, though I'm not quite certain. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Here is the next (minor) update of the library (ZIP file updated at <http://www.highscore.de/boost/gsoc2010/process.zip>; changes in Subversion at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/>). What has been changed? * The constructor of boost::process::handle supports an optional parameter to specify whether a handle should be automatically closed (by default the handle is closed). boost::process::behavior::inherit passes "dont_close" to the constructor to avoid that a handle inherited by a child process is closed by the parent process (as the handle is inherited it is a handle in use by the parent process and must not be closed). The function boost::process::create_child() does not explicitly close handles anymore but simply lets them go out of scope. * The stream behaviors don't return a std::pair<handle, handle> anymore. There is now a new struct called boost::process::stream_ends which has two member variables child and parent. Now there is no need anymore to remember whether first or second was the child's end. I'd appreciate if you could comment on the changes. Especially have a look at: * the enumeration close_type in boost::process::handle (see <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/boost/process/handle.hpp>). Is there a better name for the enumeration? Are there better names for the values in the enumeration? I don't like "do_close" but then the name "close" is already used at too many other places. As this is a minor update there will be some more changes soon (hopefully this weekend). Boris
Here is another update of the library (again ZIP file updated at <http://www.highscore.de/boost/gsoc2010/process.zip>; again changes in Subversion at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/>). What has been changed? * The library supports now configuring any streams (not only standard streams but whatever is supported on a platform). This is done through the new boost::process::context::streams array which replaces the stdin_behavior, stdout_behavior and stderr_behavior member variables. At the same time boost::process::child has been changed: This class doesn't provide the methods get_stdin(), get_stdout() and get_stderr() anymore but a single method called get_handle(). This new method expects a stream identifier as parameter and returns a boost::process::handle. It does not return a boost::process::istream or boost::process::ostream anymore as Boost.Process doesn't necessarily know if a stream is an input or output stream. This has the additional advantage though that those who like to use asynchronous I/O don't get a stream anymore (as they don't need one). Now you get a handle and decide yourself whether you want to initialize a stream (for synchronous I/O) or a pipe (for asynchronous I/O). * Stream behaviors don't expect a boolean anymore to differentiate between input and output streams. A new enumeration called boost::process::stream_type has been defined to improve readability. Furthermore stream behaviors like pipe or named_pipe provide a new constructor to explicitly specify whether a child's input or output stream should be configured. This is required on platforms which support configuring arbitrary many streams (which means POSIX :). * A bug was fixed in boost::process::detail::basic_status_service which caused problems when waiting asynchronously for child processes to terminate on POSIX platforms. Again I'd appreciate if you could comment on the changes. Especially have a look at: * boost::process::stream_type (in boost/process/stream_type.hpp) and boost::process::stream_id (in boost/process/stream_id.hpp). These types and files are new. * the new constructors in boost::process::behavior::pipe, boost::process::behavior::named_pipe and boost::process::behavior::null which expect a parameter of type boost::process::stream_type. They allow developers to explicitly specify whether an input or output stream is configured as the setting overrides the parameter passed to operator() (have a look eg. at the test case called "test_posix" in <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/test/child.cpp> to see why this is required). * the class boost::process::context. There is no stdin_behavior, stdout_behavior and stderr_behavior anymore but a new array called streams. * the class boost::process::child. As boost::process::context was changed to support basically arbitrary many streams boost::process::child had to be updated to provide access to them. * the implementation of boost::process::create_child() (especially the POSIX code). As boost::process::context looks differently now boost::process::create_child() had to be updated. * the test cases in <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/libs/process/test/child.cpp> to see how boost::process::context and boost::process::child are used now. The POSIX test cases in this file also use the new streams array to configure streams with file descriptors greater than 2 (search for "test_posix"; this is now definitely easier than before as no user-defined context class is required anymore). What still has to be done? * boost::process::context class should provide a boost::function member variable to bind a callback function instead of requiring developers to define a subclass and override setup(). * The standard constructor of boost::process::context still tries to inherit standard streams (which is a problem for Windows GUI applications which don't have standard streams). * It might make sense to pass a string to boost::process::create_child() to specify command line options (instead of storing them one by one in a vector). * It might be possible to somehow pass options to CreateProcess() on Windows (which is used by boost::process::create_child()). * Examples and documentation must be updated. Boris
On Sep 19, 2010, at 4:12 PM, Boris Schaeling wrote:
* It might make sense to pass a string to boost::process::create_child() to specify command line options (instead of storing them one by one in a vector).
I've only been lightly following the discussion (I'm interested in the library, but presently swamped with other things), but this caught my eye. You might find boost::assign::list_of and friends to be useful for dealing with the "storing them one by one in a vector" dance.
On Sun, 19 Sep 2010 23:25:17 +0200, Kim Barrett <kab.conundrums@verizon.net> wrote:
On Sep 19, 2010, at 4:12 PM, Boris Schaeling wrote:
* It might make sense to pass a string to boost::process::create_child() to specify command line options (instead of storing them one by one in a vector).
I've only been lightly following the discussion (I'm interested in the library, but presently swamped with other things), but this caught my eye. You might find boost::assign::list_of and friends to be useful for dealing with the "storing them one by one in a vector" dance.
Thanks, but boost::assign::list_of is already used (at least in my sample programs :). The idea here is that someone might want to create a child process with: create_child("program", "--lots -of /command 'line options'"); Whether this is worth to be supported by the library out of the box I don't know (as you are right boost::assign::list_of helps here). Boris
Here is another update of the library (ZIP file updated at <http://www.highscore.de/boost/gsoc2010/process.zip>; documentation updated at <http://www.highscore.de/boost/gsoc2010/>; changes in Subversion at <http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/>). I expect this to be the last update for now (as it looks like discussions have calmed down a bit and I basically went through the major changes people had asked for). There might be still some changes to come but this is something we can of course handle in an official review. What has been changed in this version? * boost::process::context provides a boost::function member variable to bind a callback function instead of requiring developers to define a subclass and override setup(). * On Windows a bug in boost::process::status has been fixed. * Examples and documentation have been updated. Thanks for all of your feedback since mid August! Boris
Hi, I was reading this last version documentation and just wanted a precision : If I want my parent process to work but to be "notified" by the end of a child process execution, whatever the reason (runtime error, unmanged exception etc) ... ...if I understood correctly, I should use boost::process::status::async_wait() to achive this. Is that correct? Thanks for your work. On Thu, Oct 7, 2010 at 01:06, Boris Schaeling <boris@highscore.de> wrote:
Here is another update of the library (ZIP file updated at < http://www.highscore.de/boost/gsoc2010/process.zip>; documentation updated at <http://www.highscore.de/boost/gsoc2010/>; changes in Subversion at < http://svn.boost.org/svn/boost/sandbox/SOC/2010/process/>).
I expect this to be the last update for now (as it looks like discussions have calmed down a bit and I basically went through the major changes people had asked for). There might be still some changes to come but this is something we can of course handle in an official review.
What has been changed in this version?
* boost::process::context provides a boost::function member variable to bind a callback function instead of requiring developers to define a subclass and override setup().
* On Windows a bug in boost::process::status has been fixed.
* Examples and documentation have been updated.
Thanks for all of your feedback since mid August!
Boris
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Thu, 07 Oct 2010 15:05:21 +0200, Klaim <mjklaim@gmail.com> wrote: Hi Klaim,
If I want my parent process to work but to be "notified" by the end of a child process execution, whatever the reason (runtime error, unmanged exception etc) ... ...if I understood correctly, I should use boost::process::status::async_wait() to achive this. Is that correct?
yes. You can also use boost::process::status::wait() if you want to wait synchronously (this method blocks). Boris
[...]
yes. You can also use boost::process::status::wait() if you want to wait synchronously (this method blocks).
As I said that the parent process have to work while wanting to be notified of children end, I guess that's not an option for me. async_wait() and wait() will work with any kind of child end of execution? Event unmanaged exception, manual kill etc? Thanks for the confirmation. Joel Lamotte.
On Fri, 08 Oct 2010 09:12:40 +0200, Klaim <mjklaim@gmail.com> wrote:
[...]async_wait() and wait() will work with any kind of child end of execution? Event unmanaged exception, manual kill etc?
Yes, as far as I know they should as they are based on wait()/waitpid() on POSIX and WaitForMultipleObjects()/WaitForSingleObject() on Windows. Boris
That's perfect. Thanks again for your work. I've worked with old version of boost::process but it seemed totally broken at the time. This version seems really simpler and actually works. Can't wait for it to be out in a boost release. :) On Fri, Oct 8, 2010 at 21:46, Boris Schaeling <boris@highscore.de> wrote:
w they should as they are based on wait()/waitpid() on POSIX and WaitForMu
Hello Boris, To portably use boost::process::behavior::inherit, it should call in it's constructor: #if defined(BOOST_WINDOWS_API) if(h_.native()) { if (!SetHandleInformation( h_.native() , HANDLE_FLAG_INHERIT , HANDLE_FLAG_INHERIT)) BOOST_PROCESS_THROW_LAST_SYSTEM_ERROR( "SetHandleInformation() failed"); } #endif Otherwise the following use case fails on Windows to redirect output to a file: void inherit_test (std::string exe, const args& a, std::string out, std::string err) { boost::iostreams::file_descriptor_sink stdOutFile; boost::iostreams::file_descriptor_sink stdErrFile; boost::process::context ctx; if (!out.empty()) { stdOutFile.open(out.c_str()); ctx.streams[boost::process::stdout_id] = boost::process::behavior::inherit(stdOutFile.handle()); } if (!standardErrorFile.empty()) { stdErrFile.open(err.c_str()); ctx.streams[boost::process::stderr_id] = boost::process::behavior::inherit(stdErrFile.handle()); } boost::process::child c = boost::process::create_child(exe, a, ctx); c.wait(); } Jeff
On Fri, 29 Oct 2010 20:30:26 +0200, Jeff Flinn <TriumphSprint2000@hotmail.com> wrote: Hi Jeff,
[...]Otherwise the following use case fails on Windows to redirect output to a file:
void inherit_test (std::string exe, const args& a, std::string out, std::string err) { boost::iostreams::file_descriptor_sink stdOutFile; boost::iostreams::file_descriptor_sink stdErrFile;
thanks for your feedback and the test case! I think you are right with everything you said. I only think because I can't reproduce the problem. :-/ Not because there is no problem but because I get a linker error: No single method of boost::iostreams::file_descriptor_sink is found. My boost_iostreams-vc90-mt-gd-1_43.dll indeed provides only boost::iostreams::file_descriptor but neither _sink nor _source. I can't find any hint in the Boost.Iostreams documentation what I have to do to build these classes, too (Boost 1.43 and VS2008). Everything works fine on my Linux machine though (and your bug report is of course Windows-specific). How did you build Boost.Iostreams? Boris
[...]
Boris Schaeling wrote:
On Fri, 29 Oct 2010 20:30:26 +0200, Jeff Flinn <TriumphSprint2000@hotmail.com> wrote:
Hi Jeff,
[...]Otherwise the following use case fails on Windows to redirect output to a file:
void inherit_test (std::string exe, const args& a, std::string out, std::string err) { boost::iostreams::file_descriptor_sink stdOutFile; boost::iostreams::file_descriptor_sink stdErrFile;
thanks for your feedback and the test case! I think you are right with everything you said. I only think because I can't reproduce the problem. :-/ Not because there is no problem but because I get a linker error: No single method of boost::iostreams::file_descriptor_sink is found. My boost_iostreams-vc90-mt-gd-1_43.dll indeed provides only boost::iostreams::file_descriptor but neither _sink nor _source.
I can't find any hint in the Boost.Iostreams documentation what I have to do to build these classes, too (Boost 1.43 and VS2008). Everything works fine on my Linux machine though (and your bug report is of course Windows-specific). How did you build Boost.Iostreams?
I'm using boost 1.44. The complete command line(s) that we use to build static boost libs on windows with MSVC8 are: bjam link=static runtime-link=static debug -sZLIB_SOURCE=F:\boost\ZLib-1.2.5 stage bjam define=_SECURE_SCL=0 link=static runtime-link=static release -sZLIB_SOURCE=F:\boost\ZLib-1.2.5 stage The zlib stuff shouldn't matter. And I've used the following include: #include <boost/iostreams/device/file_descriptor.hpp> Thanks, Jeff
On Sat, 30 Oct 2010 17:10:13 +0200, Jeff Flinn <TriumphSprint2000@hotmail.com> wrote:
[...]I'm using boost 1.44. The complete command line(s) that we use to build static boost libs on windows with MSVC8 are:
I upgraded now to 1.44. For whatever reason the linker error disappeared. The bug (and another one reported to me by email) is now fixed. The latest version of the library can be downloaded as usual from <http://www.highscore.de/boost/gsoc2010/process.zip>. Thanks, Boris
[...]
participants (18)
-
Alexander Lamaison -
Boris Schaeling -
Daniel Trebbien -
Gevorg Voskanyan -
Ilya Sokolov -
Jeff Flinn -
Jeremy Maitin-Shepard -
John B. Turpish -
John Bellone -
Kim Barrett -
Klaim -
Mathias Gaunard -
Roland Bock -
Stewart, Robert -
vicente.botet -
Vladimir Prus -
Wolfgang Fertsak -
Wolfgang Fertsak