Ticket #5008 enhancement closed fixed

Opened 2 years ago

Last modified 2 years ago

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

1

Changed 2 years ago by mwh

  • branch set to branches/close-dev-null-5008
  • branch_author set to mwh

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

2

Changed 2 years ago by mwh

  • keywords review added
  • branch branches/close-dev-null-5008 deleted
  • branch_author mwh deleted

I applied the patch in [31555]

3

Changed 2 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.

4

Changed 2 years ago by mwh

  • owner mwh deleted
  • keywords review added

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

5

Changed 2 years ago by exarkun

 build results

6

Changed 2 years ago by exarkun

  • keywords review removed
  • owner set to mwh

Looks good, please merge. Thanks

7

Changed 2 years ago by mwh

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

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