Skip to content

Return Process::Waiter with pid in Process.detach#4566

Merged
headius merged 1 commit intojruby:masterfrom
davishmcclurg:davishmcclurg/wait-thread-pid
Apr 19, 2017
Merged

Return Process::Waiter with pid in Process.detach#4566
headius merged 1 commit intojruby:masterfrom
davishmcclurg:davishmcclurg/wait-thread-pid

Conversation

@davishmcclurg
Copy link
Copy Markdown
Contributor

This adds a pid method to threads returned by Process.detach,
as described in the open3.rb documentation. The thread is a
Process::Waiter to be consistent with standard Ruby.

I haven't worked in Java much before, so please let me know if there's
anything I'm missing.

Before:

>> JRUBY_VERSION
=> "9.1.4.0"
>> pid = Process.spawn('sleep 10')
=> 58487
>> thr = Process.detach(_)
=> #<Thread:0x45752059@(irb):3 run>
>> thr.pid
NoMethodError: undefined method `pid' for #<Thread:0x45752059@(irb):3 dead>

After:

>> JRUBY_VERSION
=> "9.1.9.0-SNAPSHOT"
>> pid = Process.spawn('sleep 10')
=> 59018
>> thr = Process.detach(_)
=> #<Process::Waiter:0x5acf93bb@(irb):3 run>
>> thr.pid
=> 59018

This adds a `pid` method to threads returned by `Process.detach`,
as described in the open3.rb documentation. The thread is a
`Process::Waiter` to be consistent with standard Ruby.

Before:

```
>> JRUBY_VERSION
=> "9.1.4.0"
>> pid = Process.spawn('sleep 10')
=> 58487
>> thr = Process.detach(_)
=> #<Thread:0x45752059@(irb):3 run>
>> thr.pid
NoMethodError: undefined method `pid' for #<Thread:0x45752059@(irb):3 dead>
```

After:

```
>> JRUBY_VERSION
=> "9.1.9.0-SNAPSHOT"
>> pid = Process.spawn('sleep 10')
=> 59018
>> thr = Process.detach(_)
=> #<Process::Waiter:0x5acf93bb@(irb):3 run>
>> thr.pid
=> 59018
```
@headius
Copy link
Copy Markdown
Member

headius commented Apr 19, 2017

This appears to be the same way MRI implements Waiter#pid and the PR looks fine. Merging.

@headius headius merged commit 8568572 into jruby:master Apr 19, 2017
@headius headius added this to the JRuby 9.1.9.0 milestone Apr 19, 2017
@davishmcclurg davishmcclurg deleted the davishmcclurg/wait-thread-pid branch October 8, 2019 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants