Opened 4 years ago

Closed 3 years ago

#5008 enhancement closed fixed (fixed)

Close /dev/null in twisted.internet.process._FDDetector._checkDevFDSanity explicitly

Reported by: exarkun Owned by: mwh
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch:
Author: Launchpad Bug:

Description

The current implementation lets reference counting/garbage collection close it, which may result in it being left open for longer than expected or desired.

A patch is attached to #4881 which addresses this.

Change History (7)

comment:1 Changed 3 years ago by mwh

  • Author set to mwh
  • Branch set to branches/close-dev-null-5008

(In [31553]) Branching to 'close-dev-null-5008'

comment:2 Changed 3 years ago by mwh

  • Author mwh deleted
  • Branch branches/close-dev-null-5008 deleted
  • Keywords review added

I applied the patch in [31555]

comment:3 Changed 3 years ago by exarkun

  • Keywords review removed
  • Owner set to mwh

I think one of the tests should assert that the fake file is closed. Otherwise, the change just lets the implementation close it, but doesn't verify that it does.

comment:4 Changed 3 years ago by mwh

  • Keywords review added
  • Owner mwh deleted

Fair enough. I added a test in [31559].

comment:6 Changed 3 years ago by exarkun

  • Keywords review removed
  • Owner set to mwh

Looks good, please merge. Thanks

comment:7 Changed 3 years ago by mwh

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

(In [31569]) Merge close-dev-null-5008: explicitly close /dev/null in t.i.process._FDDetector._checkDevFDSanity

Author: lewq, mwh
Reviewer: exarkun
Fixes: #5008

twisted.internet.process._FDDetector._checkDevFDSanity now explicitly closes
the file descriptor it opens rather than just waiting for GC to close it.

Note: See TracTickets for help on using tickets.