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
Unix.getpid returns wrong result #4034
Comments
Comment author: Christoph Bauer Important note: the platform is windows (mingw-version in my case). |
Comment author: @xavierleroy There are no strict equivalent to Unix's process IDs in the Win32 API, Caml uses process handles as closest equivalents of process IDs for the |
Comment author: Christoph Bauer There is a function GetCurrentProcessID. The task manager winwait.c could make use of OpenProcess, which takes the |
Comment author: @xavierleroy I keep that as a feature wish. |
Comment author: Christoph Bauer I attached an patch that seems to work:
My tests are done with the mingw version. I found the API In the original implementation closes never the hProcess-Handle |
Comment author: Christoph Bauer Please apply the patch in the directory `ocaml-3.09.2/otherlibs/win32unix'. |
Comment author: Christoph Bauer I tested the patch now a long time and had never a problem with it. This entry could be counted as bug because of the process handle leak under windows... |
Comment author: @xavierleroy I tried the patch but runs into errors returned by OpenProcess. let ic = Unix.open_process_in "dir c:\\" in
begin try
while true do
print_string (input_line ic); print_newline()
done
with End_of_file -> ()
end;
ignore (Unix.close_process_in ic) Don't you get an error on close_process_in? |
Comment author: Christoph Bauer I guess you are right. With your example I get the error sometimes. So a Unix.sleep and now I get the error more reliably: let ic = Unix.open_process_in "dir c:\\" in
begin try
while true do
print_string (input_line ic); print_newline()
done
with End_of_file -> ()
end;
Unix.sleep 1;
ignore (Unix.close_process_in ic);; There are two possible solutions. The simple one: The second solution could be a internal table that stores for each pid the process information. Then the job of waitpid would be to free the internal handle. The table could be realized as a simple list or a simple hash table. I try to implement the second approach. |
Comment author: Christoph Bauer The new patch implements a pid table of PROCESS_INFORMATION. (binary search, |
Comment author: Christoph Bauer There's a stupid bug in patch 2b. I'll prepare a new one. |
Comment author: Christoph Bauer The patch 2e was quite good. 2f is a patch against 3.10.1. |
Comment author: Christoph Bauer patch against 3.12.1. compiles with msvc. |
Comment author: Christoph Bauer patch against 4.01.0 uploaded: ocaml-getpid-4.01.0a.patch |
Comment author: gabelevi (It's very possible that I'm missing some historical information about Windows. I did notice that some functions, like GetCurrentProcessID, were introduced with Windows XP, so maybe Windows didn't used to have a notion of Process IDs). It does appear that Windows has the concept of Process IDs, in addition to Process Handles. I think it would be better if the various functions that return references to a process (like Unix.getpid and Unix.create_process) and the various functions that take a reference to a process (like Unix.waitpid and Unix.kill) used Process IDs instead of Process Handles. I have code that I am porting to Windows. While the create_process/waitpid pattern still works, there are other patterns that break. I need to manually translate Process Handles to Process IDs when I want to output the "pid" to stdout, a log file, etc. Also, I need to manually translate Process IDs to Process Handles when I read the "pid" from stdin, a file, etc. Process IDs are portable across processes in a way that Process Handles are not. So while Process Handles work well enough as an opaque pseudo-pid, I think Process IDs would work just as well and would prevent unnecessary translation when outputting and inputting pids. (For reference, here is the change I had to make to get some code working on Windows. Some of the code is responsible for logging pids to a file in case they need to be manually terminated later, and has to translate Process Handles to Process IDs. The code that reads these Process IDs needs to translate them back into Process Handles to terminate those processes. The third affected piece of code just reports to stdout the pid of a created process. facebook/flow@276bb7f) |
This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc. |
This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc. |
As I wrote in 2007, The problem with using process ID instead of handles is:
In other words: the current implementation using handles is correct and the one using process IDs is not. So I'll close this issue. If anyone sees a workaround, please reopen. |
A related remark only : I think it would seem coherent with the treatment of file descriptors in the (If we make such changes to Unix, and to keep backward compatibility, one would need to "fork" the waitpid and functions that create processes. This could be the occasion to also allow passing extra (sometimes OS-specific) flags when creating processes, as we do when opening files.) |
Original bug ID: 4034
Reporter: Christoph Bauer
Status: acknowledged (set by @xavierleroy on 2006-06-10T09:11:56Z)
Resolution: open
Priority: normal
Severity: feature
OS: Windows
Version: 3.09.2
Category: otherlibs
Tags: patch
Monitored by: @nojb @dra27 @alainfrisch
Bug description
Unix.getpid returns a wrong (non-existing) processid.
To reproduce:
A working implementation of getpid.c could be
File attachments
The text was updated successfully, but these errors were encountered: