[llvm-commits] [llvm] r81826 - in /llvm/trunk: include/llvm/System/Program.h lib/System/Unix/Program.inc lib/System/Win32/Program.inc

[Daniel Dunbar]

On Mon, Sep 21, 2009 at 11:24 PM, Mikhail Glushenkov
<foldr at codedgers.com> wrote:
> Hi Daniel,
>
> On Mon, Sep 21, 2009 at 09:43:20PM -0700, Daniel Dunbar wrote:
>> Hi Mikhail,
>>
>> This isn't safe, for exactly the same reasons as I fixed it before here:
>>   http://llvm.org/viewvc/llvm-project?view=rev&revision=77953
>> and it breaks ExecuteAndWait in exactly the same way. I'm reverting it
>> for now. See http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090727/083264.html
>
> Sorry, I haven't seen your previous e-mail. I don't quite understand what causes
> the race condition, though...

I'm not sure if its a race condition or just not supported on some
versions of Windows, but in general once you have a handle you should
hold that handle for as long as you want to use it. Even if the API
provides something to get the handle from a PID, its probably best to
avoid it. I think the problem in this case is either that the process
dies before you try to grab the handle again (via the pid), or its
just flat out not supported on my version of Windows (XP SP2).

>> If the motivation is supporting GetProcessId pre XP then this should
>> be recorded in the Program object.
>
> "this" == PID? Yes, I guess that's better than any of the tricks from [1].

Yes.

>> I also see no reason to make a Program object copyable -- what would this
>> mean? The program object should essentially be opaque, it is implemented by
>> the System layer, which may well be non-copyable.
>
> Well, if the Program object is just a PID, why not make it copyable? Clients may
> want to store it somewhere (e.g. STL containers). Alternatively, we may add a
> way to construct a Program from a PID (since there is already a GetPid()
> method).

Because it isn't portable to assume that the system can implement the
Program API in terms of a PID (where pid == uint32_t, or so), nor can
one rely on the system to supply a method to populate all the data in
a Program from just a PID. Clients can put a Program* in an STL
container just as easily as a PID.

 - Daniel

>> I know we don't currently have a good way of running tests on Windows,
>> but please verify that ExecuteAndWait doesn't break when modifying
>> this code...
>
> Sorry for that.
>
> [1] http://www.codeproject.com/KB/threads/GettingProcessID.aspx
>