Issue2221

Title pretty much explains the problem. After studying this a bit, I was able to implement getting pid so that it works at least on Windows 7 and Linux Mint. The resulting code is at the end of this description and it can be tested by running the attached script that contains also simple code to verify the results.

If there's interest, I can take a look at turning this into a pull request. Final solution ought to have error handling at least when accessing private fields. I would assume it's better to return None than raise an exception on error.

----------8<------------------8<----------------------


import os

if os.sep == '/':

    def get_pid(process):
        field = process.getClass().getDeclaredField('pid')
        field.setAccessible(True)
        return field.getLong(process)

else:

    import ctypes
    GetProcessId = ctypes.cdll.kernel32.GetProcessId
    GetProcessId.argtypes = (ctypes.c_long,)

    def get_pid(proces):
        field = process.getClass().getDeclaredField('handle')
        field.setAccessible(True)
        handle = field.getLong(process)
        return GetProcessId(handle)

Colleague tested this on OSX. His comments:

1) Code works fine.

2) The description isn't too clear how to test this. The correct approach is downloading and running the attached jython_pid.py. The code in the description doesn't work with normal subprocess.Popen.

3) Windows version of `get_pid` has argument `proces` (typo). The test code works on Windows, though, because there is `process` available in the global scope.

Interesting use of ctypes. I believe the most portable way to do this is with JMX however.

Let's implement a robust scheme. As long as we have error handling, it will be better than simply returning None, which remains valid for Jython if we cannot determine the value. (Or perhaps we raise an exception - that could be reasonable too). Then once we have JEP 102 (http://openjdk.java.net/jeps/102) available on Java 9, we can use that instead in a future release of 2.7.x (where x > 0, probably greater than 1)

Personally I just prefer a patch, including tests, attached to this bug. PRs on Bitbucket are just a mess; although it's possible that PRs against the GitHub mirror work better. To be seen shortly.

Please add any tests to test_subprocess_jy.py

Thanks!

Forgot to mention that both POSIX and Windows solutions are based on answers to SO question http://stackoverflow.com/questions/4750470/how-to-get-pid-of-process-ive-just-started-within-java-program and on http://www.golesny.de/p/code/javagetpid linked from there. Getting pid on POSIX and handle on Windows via reflection is pretty trivial, assuming we don't need to care about security managers. Converting a handle to a pid looked pretty complicated in the examples, but after I found out Kernel32 has GetProcessId for exactly that purpose it turned out to be pretty simple too.

I'm nor certain is the above the best way to get pid but at least it is pretty promising. I don't have experience using JMX but could take a look at that too. Do you have pointers where to look?

Regardless the approach we choose, I think not getting a pid shouldn't cause an exception at least directly when the process is started. We could turn Popen.pid into a property and raise an exception if it is accessed, but I'm not sure is that worth the effort. Just setting it to None would be backwards compatible with earlier Jython versions.

After we are ready with the overall design, I can take a look at creating a pull request on GitHub. My experience from them both when reviewing and submitting has been fairly positive. Would it be OK for tests to run a process like `python -c "import os; print os.getpid()"` and compare the output to Popen.pid? In other words, can be assume that devs running tests have python on PATH?

Here is a pull request: https://github.com/jythontools/jython/pull/12

Merged in changesets 593d5c73444b and 2ffae23d1e07.