Opened 9 years ago

Closed 6 years ago

#1246 defect closed fixed (fixed)

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 (1)

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

Download all attachments as: .zip

Change History (17)

Changed 9 years ago by hagna

comment: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.

comment:2 Changed 8 years ago by hypatia

  • Cc hypatia removed
  • Component set to core
  • Owner changed from hypatia to edsuom

comment:3 Changed 6 years ago by thijs

  • Cc thijs added
  • Keywords review added

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

comment:4 Changed 6 years ago by thijs

  • Owner edsuom deleted
  • Priority changed from high to normal

comment:5 follow-up: Changed 6 years ago by exarkun

  • Keywords review removed
  • Owner set to thijs

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()

comment:6 Changed 6 years ago by thijs

  • Status changed from new to assigned

comment:7 Changed 6 years ago by thijs

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

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

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

  • Keywords review added
  • Owner thijs deleted
  • 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.

comment:9 follow-up: Changed 6 years ago by exarkun

  • Keywords 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!

comment:10 Changed 6 years ago by thijs

  • Status changed from new to assigned

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

  • Keywords review added
  • Owner changed from thijs to exarkun
  • Status changed from assigned to new

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')

comment:12 follow-up: Changed 6 years ago by exarkun

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

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

Replying to exarkun:

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

Fixed in r24645

comment:14 Changed 6 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to thijs

Looks good, please merge.

comment:15 Changed 6 years ago by thijs

  • Resolution set to fixed
  • Status changed from new to closed

(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

comment:16 Changed 3 years ago by <automation>

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