Ticket #1246 defect closed fixed

Opened 9 years ago

Last modified 6 years ago

reactor.callWhenRunning is not in the Using Processes document

Reported by: hagna Owned by:
Priority: normal Milestone:
Component: core Keywords: documentation
Cc: hagna, thijs Branch: branches/callwhenrunning-doc-1246
(diff, github, buildbot, log)
Author: thijs Launchpad Bug:

Description


Attachments

example.py Download (2.4 KB) - added by hagna 9 years ago.

Change History

Changed 9 years ago by hagna

1

  Changed 9 years ago by hagna

The way to get rid of a race condition where spawnprocess is called before the
signal handler is installed is to use reactor.callWhenRunning(spawnprocess ...).
 The examples in "Using Processes" don't mention the race condition or the
solution.  

Here is an example that doesn't use callWhenRunning.  It would work fine if it
was using callWhenRunning.

Changed 9 years ago by hagna

2

  Changed 8 years ago by hypatia

  • owner changed from hypatia to edsuom
  • cc hypatia removed
  • component set to core

3

  Changed 6 years ago by thijs

  • keywords documentation, review added; documentation removed
  • cc thijs added

Shall I create a patch that adds this to the core docs? Can anyone review this example?

4

  Changed 6 years ago by thijs

  • owner edsuom deleted
  • priority changed from high to normal

5

follow-up: ↓ 8   Changed 6 years ago by exarkun

  • owner set to thijs
  • keywords documentation added; documentation, review removed

This example is rather complex. A simpler example would be more like this (untested):

from twisted.internet.utils import getProcessOutput
from twisted.internet import reactor

d = getProcessOutput('/bin/echo', ['/bin/echo', 'foo'])
def cbOutput(output):
    print output
    reactor.stop()
d.addCallback(cbOutput)
reactor.run()

This would better be written (for the time being) as

from twisted.internet.utils import getProcessOutput
from twisted.internet import reactor

def getOutput():
    d = getProcessOutput('/bin/echo', ['/bin/echo', 'foo'])
    def cbOutput(output):
        print output
        reactor.stop()
    d.addCallback(cbOutput)
reactor.callWhenRunning(getOutput)
reactor.run()

6

  Changed 6 years ago by thijs

  • status changed from new to assigned

7

  Changed 6 years ago by thijs

  • branch set to branches/callwhenrunning-doc-1246
  • author set to thijs

(In [24448]) Branching to 'callwhenrunning-doc-1246'

8

in reply to: ↑ 5   Changed 6 years ago by thijs

  • owner thijs deleted
  • keywords documentation, review added; documentation removed
  • status changed from assigned to new

Replying to exarkun:

This example is rather complex. A simpler example would be more like this (untested): ...

I added your example in a branch with the terse description hagna offered. Let me know how it looks and fits.

9

follow-up: ↓ 11   Changed 6 years ago by exarkun

  • keywords documentation added; documentation, review removed
  • owner set to thijs

spawnprocess in the prose should be capitalized properly and would probably benefit from being an API link. The reference to reactor.callWhenRunning following it could be an API link too.

I'd suggest dropping the mention of a "signal handler" and just talk about how calling reactor.spawnProcess before calling reactor.run can lead to a race where process exit isn't noticed (and a warning emitted). Then it becomes more clear how using reactor.callWhenRunning helps. This should also help make the statement "The way to get rid of a race condition..." more clear, because the actual race condition will have been defined in the document, which it currently isn't.

Also, my example contained a bug - getProcessOutput does not take argv[0] as part of the argument list, but I included it because I always forget about that inconsistency.

Thanks!

10

  Changed 6 years ago by thijs

  • status changed from new to assigned

11

in reply to: ↑ 9   Changed 6 years ago by thijs

  • owner changed from thijs to exarkun
  • status changed from assigned to new
  • keywords documentation, review added; documentation removed

Replying to exarkun:

spawnprocess in the prose should be capitalized properly and would probably benefit from being an API link. The reference to reactor.callWhenRunning following it could be an API link too. I'd suggest dropping the mention of a "signal handler" and just talk about how calling reactor.spawnProcess before calling reactor.run can lead to a race where process exit isn't noticed (and a warning emitted). Then it becomes more clear how using reactor.callWhenRunning helps. This should also help make the statement "The way to get rid of a race condition..." more clear, because the actual race condition will have been defined in the document, which it currently isn't.

Rewrote it in r24642, putting it up for review.

Also, my example contained a bug - getProcessOutput does not take argv[0] as part of the argument list, but I included it because I always forget about that inconsistency.

What does this mean, something like this?

- d = getProcessOutput('/bin/echo', ['/bin/echo', 'foo'])
+ d = getProcessOutput('/bin/echo')

12

follow-up: ↓ 13   Changed 6 years ago by exarkun

It means d = getProcessOutput('/bin/echo', ['foo'])

13

in reply to: ↑ 12   Changed 6 years ago by thijs

Replying to exarkun:

It means d = getProcessOutput('/bin/echo', ['foo'])

Fixed in r24645

14

  Changed 6 years ago by exarkun

  • owner changed from exarkun to thijs
  • keywords documentation added; documentation, review removed

Looks good, please merge.

15

  Changed 6 years ago by thijs

  • status changed from new to closed
  • resolution set to fixed

(In [24652]) Merge callwhenrunning-doc-1246: add a new sample for reactor.callWhenRunning in the Using Processes document.

Author: hagna, exarkun, thijs Reviewer: exarkun Fixes: #1246

16

  Changed 3 years ago by <automation>

  • owner thijs deleted
Note: See TracTickets for help on using tickets.