Add thisPid() and Pid.getThis()

[nordlow]

This adds a typesafer version of thisProcessID named thisProcessPid.

See discussion at http://forum.dlang.org/post/pziiwlhoyyolgvvljgcg@forum.dlang.org.

[CyberShadow]

thisProcessPid? As in "This process process identifier"?

[CyberShadow]

How about a Pid.getThis() static method, to complement Thread.getThis()?

[nordlow]

Ah, I guess thisPid is better, of course :)

[schuetzm]

s/that/than/

[schuetzm]

"," after "for instance"

[DmitryOlshansky]

LGTM

[CyberShadow]

I think we need either thisPid or Pid.getThis, not both. This isn't Ruby :)

[nordlow]

If we remove thisPid we should add a reference to Pid.getThis in the documentation for thisProcessID and perhaps also for Thread.getThis. Perhaps it's even motivated to deprecate thisProcessID with a warning.

[schveiguy]

The allocation per call for (essentially) an immutable seems wasteful.

But the whole idea here seems wasteful. The only places Pid is used is in wait and kill. The only useful subset of those for the current process is kill on posix systems for a specific signal type.

Do we need to add a whole new API call for this? You can do core.sys.posix.signal.kill(thisProcessId, sig) instead, no?

[nordlow]

@schveiguy, I agree that the allocation in Pid.getThis is wasteful.

Can the result of Pid.getThis() be changed during the life-time of a process?

If not, shouldn't the result of Pid.getThis() be cached?

@schveiguy, I thought the policy of Phobos was to, when possible, wrap untyped interfaces (UNIX-style int descriptors) into typed (Pid) for increased code correctness..

[CyberShadow]

Can the result of Pid.getThis() be changed during the life-time of a process?

Yes, if the process is forked.

[nordlow]

@schveiguy, if Pid is wasteful, why was it introduced in the first place?

[DmitryOlshansky]

@schveiguy The fault seems to be with Pid being a class. Any chance to undo that w/o deprecating the whole thing?

[nordlow]

@DmitryOlshansky changing it to a struct would be a good solution, if it doesn't break too much code. Deprecation phase?

[DmitryOlshansky]

@nordlow typesafety I guess. Effectively it seems to be an opaque type for whatever is the platform specific process handle/integer

[schveiguy]

The point of making it a class, I think, is to avoid leaking the handle (specifically for Windows).

I'm curious if we couldn't generate the Handle each time it's needed, and then close the handle within the same function (kill, wait, etc.), and then make this purely a value type struct. I haven't looked at this code in a while, so I can't remember how Windows process management works. If you are performing an operation like kill or wait, is the extra cost of looking up the Handle going to impact performance?

Another option is to create an overload for kill (killMe?) that works on the current process ID for appropriate platforms. Are there other uses for having a Pid for the current process?

ping @kyllingstad

[schveiguy]

Yes, if the process is forked.

Yes, but if you do this:

auto pid = thisPid;
fork();

you have the same problem. The trick would be somehow to detect a change in the current process and update the singleton. I admit, I don't like this idea at all :)

[kyllingstad]

@schveiguy:

The point of making it a class, I think, is to avoid leaking the handle (specifically for Windows).

I'm curious if we couldn't generate the Handle each time it's needed, and then close the handle within the same function (kill, wait, etc.), and then make this purely a value type struct.

Well, this is funny. It was, in fact, you who convinced me to make Pid a reference type, exactly to avoid this kind of solution. :-) In my original (POSIX-only) draft implementation it was a struct—a simple wrapper around a process ID number.

To everyone else: The reason Pid is a class is to encapsulate the Windows HANDLE in a way that it can be safely invalidated (by wait).

I don't think there is a compelling use case for thisPid here. Killing a process by signal is POSIX specific, and it is therefore reasonable to expect users to call POSIX functions directly.

[kyllingstad]

In other words, I don't think this pull request should be merged.

[schveiguy]

It was, in fact, you who convinced me to make Pid a reference type

As I typed it, I knew deep down I was going to be at fault here :)

The thing about Pid construction right now is that it only happens when creating a process. So we don't have to worry about the performance of using the heap -- process creation is heavy anyway. But accessing a property which is 'this process' shifts the overhead.

Are there any other use cases? If not, let's just close this.

[DmitryOlshansky]

The reason Pid is a class is to encapsulate the Windows HANDLE in a way that it can be safely invalidated (by wait).

I'll bite - there is INVALID_HANDLE on windows, what's the problem with invalidation?

[schveiguy]

there is INVALID_HANDLE on windows, what's the problem with invalidation

If Pid is a struct, then one can close the handle by terminating the process in one instance, but it remains unchanged in another instance. Then you are operating on essentially free'd memory (which may have been allocated again) with the second instance:

auto pid2 = pid1;
pid1.wait();
pid2.wait(); // what does this do?

Really, you could do RefCounted, but I don't see how it shifts the problem enough to be worth it.

[DmitryOlshansky]

Really, you could do RefCounted, but I don't see how it shifts the problem enough to be worth it.

Got it. This looks like overkill.

[nordlow]

killMe would be nice, as this is the reason why I want this.

I'm using it to suspend a process from within the process itself.

[kyllingstad]

@nordlow:

killMe would be nice, as this is the reason why I want this.

Again, I don't see how this warrants a dedicated Phobos function. You're trying to do a POSIX-specific thing here, so what's wrong with using POSIX functions?

import core.sys.posix.signal, std.process;
kill(thisProcessID, SIGTSTP);

You can even avoid pulling in std.process altogether:

import core.sys.posix.signal, core.sys.posix.unistd;
kill(getpid(), SIGTSTP);

[kyllingstad]

Closing, since there seems to be consensus for doing so.