Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Need ProcessMonitor windows support #1203

Closed
Triangle345 opened this issue Sep 7, 2016 · 10 comments
Closed

Need ProcessMonitor windows support #1203

Triangle345 opened this issue Sep 7, 2016 · 10 comments
Labels
help wanted Extra attention is needed

Comments

@Triangle345
Copy link

Currently the process monitor is only written/works for linux.

For windows one would need to use the createprocess call:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx

@Triangle345 Triangle345 changed the title Need ProcessMonitor for windows Need ProcessMonitor windows support Sep 7, 2016
@lispmeister
Copy link
Member

Any PR will be gratefully accepted.

@SeanTAllen
Copy link
Member

Ideally, I think this should be delayed until #1014 is completed. Then, we can make sure that they are feature equivalent. However, #1014 has been open for a long time so I don't think that is actually reasonable. Rather, whoever implements ProcessMonitor support for Windows needs to be aware of the issue that is still outstanding in the *nixen version.

@Triangle345
Copy link
Author

I started working on this somewhat.. at the very least I can contribute a thin c wrapper for windows processes. Then maybe start incorporating it into proc monitor.

@jemc
Copy link
Member

jemc commented Feb 19, 2017

@Triangle345 Thanks!

@Triangle345
Copy link
Author

How should we test this functionality. Should I add a windows section to the test file? I notice it is currently linux specific.

@SeanTAllen
Copy link
Member

Yes. If tests that are currently Linux and OSX only pass then we can feel confident that Windows support is working.

@SeanTAllen
Copy link
Member

@Triangle345 once that is in place, if you are interested in further extending ProcessMonitor support then you could revive this PR to bring async write buffering support (which would then be across platforms). Its old and somewhat dated but could serve as a good basis for additional work.

#1014

@cquinn
Copy link
Contributor

cquinn commented Nov 24, 2017

It looks like #2186 solves the problem identified in #1014, so that should be out of the way.

@SeanTAllen
Copy link
Member

SeanTAllen commented Dec 13, 2019

@cquinn I think this can be closed, do you agree?

@cquinn
Copy link
Contributor

cquinn commented Dec 13, 2019

Yes, I think so. ProcessMonitor on Windows could be improved, but it works OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants