Opened 15 years ago

Closed 15 years ago

Last modified 5 years ago

#2123 defect closed fixed (fixed)

t.c.t.test_manhole.ManholeLoopbackStdio.testControlBackslash fails intermittently

Reported by: David Reid Owned by:
Priority: highest Milestone:
Component: conch Keywords:
Cc: Branch:
Author:

Description

I should say this "passes intermittently" because it seems to fail most of the time on my powerbook.

===============================================================================
[FAIL]: twisted.conch.test.test_manhole.ManholeLoopbackStdio.testControlBackslash

Traceback (most recent call last):
  File "/Users/dreid/projects/Twisted/trunk/twisted/conch/test/test_manhole.py", line 179, in gotClearedLine
    self._assertBuffer(
  File "/Users/dreid/projects/Twisted/trunk/twisted/conch/test/test_recvline.py", line 380, in _assertBuffer
    str(receivedLines[max(0, i-1):i+1]) +
twisted.trial.unittest.FailTest: ['>>> cancelled line'] != ['']
-------------------------------------------------------------------------------

Change History (10)

comment:1 Changed 15 years ago by David Reid

Type: enhancementdefect

comment:2 Changed 15 years ago by David Reid

I've only observed this on OS X.

comment:3 Changed 15 years ago by Jean-Paul Calderone

This seems to it less likely for the race to go the wrong way:

Index: twisted/conch/stdio.py
===================================================================
--- twisted/conch/stdio.py      (revision 18289)
+++ twisted/conch/stdio.py      (working copy)
@@ -52,8 +52,7 @@
             self.proto = None
 
 class ConsoleManhole(ColoredManhole):
-    def handle_QUIT(self):
-        ColoredManhole.handle_QUIT(self)
+    def connectionLost(self, reason):
         reactor.stop()
 
 def runWithProtocol(klass):

Apparently it is not a complete fix, though. Don't really understand why yet.

comment:4 Changed 15 years ago by Jean-Paul Calderone

Keywords: review added
Owner: z3p deleted
Priority: normalhighest

See #2371. The above is not a complete fix because OS X randomly drops bytes on the floor unless one does extra things to trick it into delivering them.

comment:5 Changed 15 years ago by Jean-Paul Calderone

Keywords: review removed

This isn't really up for review right now. I need to fix a couple things in the branch.

comment:6 Changed 15 years ago by Jean-Paul Calderone

Keywords: review added

Okay, now this should be fixed in ctrl+backslash-2371+2123-3

comment:7 Changed 15 years ago by therve

FYI, it's ctrl+backslash-2371+2123-4.

comment:8 Changed 15 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [19640]) Merge ctrl+backslash-2371+2123-5

Author: exarkun Reviewer: jknight, radix, therve Fixes #2371, #2123, #2378

Remove the long-since obsolete test helper SignalMixin, which set up and tore down special signal handling state. For some time, trial has taken care of this detail. (#2378)

Change the process file descriptor "connection lost" code to reverse the setNonBlocking operation done during initialization, leaving the file descriptor in the blocking state (what it probably was before it was given to the process code). This works around a bug with PTYs on OS X where the last write made to a non-blocking PTY might be lost if the process exits before the data is read from the other end of the PTY. This also corrects a (perhaps unreasonable) assumption that blocking writes may be made to stdout after twisted.internet.stdio has been used. (#2371)

Change ConsoleManhole so that it waits for connectionLost notification before stopping the reactor. This avoids the possibility that the reactor will stop before its transport has had all of its output flushed. Combined with the fix for #2371, this fixes #2123.

comment:9 Changed 11 years ago by <automation>

comment:10 Changed 5 years ago by hawkowl

Keywords: review removed

[mass edit] Removing review from closed tickets.

Note: See TracTickets for help on using tickets.