Skip to content

Commit 870d425

Browse files
spevansspevans
committed
SR-7608: Pipe() crashes when the underlying system call fails.
- Pipe does not have a failable or throwable initializer but currently calls fatalError() for all errors returned from pipe(). - If pipe() returns EMFILE or ENFILE then set the fileDescriptor to -1 for both .fileHandleForReading and .fileHandleForWriting. For all other errors call fatalError(). - TestFileHandle.swift: Remove duplicate test_pipe(). - TestPipe.swift: Add extra test to try and exhaust the process's pipes. - Test the file descriptor is valid and open for certain methods and properties to match Darwin.
1 parent 68c0749 commit 870d425

File tree

3 files changed

+75
-64
lines changed

3 files changed

+75
-64
lines changed

‎Foundation/FileHandle.swift‎

Lines changed: 45 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
// This source file is part of the Swift.org open source project
22
//
3-
// Copyright (c) 2014 - 2016 Apple Inc. and the Swift project authors
3+
// Copyright (c) 2014 - 2016, 2018 Apple Inc. and the Swift project authors
44
// Licensed under Apache License v2.0 with Runtime Library Exception
55
//
6-
// See http://swift.org/LICENSE.txt for license information
7-
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
6+
// See https://swift.org/LICENSE.txt for license information
7+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
88
//
99

1010
import CoreFoundation
@@ -16,9 +16,12 @@ import Glibc
1616
#endif
1717

1818
open class FileHandle : NSObject, NSSecureCoding {
19-
internal var _fd: Int32
20-
internal var _closeOnDealloc: Bool
21-
internal var _closed: Bool = false
19+
private var _fd: Int32
20+
private var _closeOnDealloc: Bool
21+
22+
open var fileDescriptor: Int32 {
23+
return _fd
24+
}
2225

2326
open var readabilityHandler: ((FileHandle) -> Void)? = {
2427
(FileHandle) -> Void in NSUnimplemented()
@@ -40,10 +43,11 @@ open class FileHandle : NSObject, NSSecureCoding {
4043
}
4144

4245
internal func _readDataOfLength(_ length: Int, untilEOF: Bool) -> Data {
46+
precondition(_fd >= 0, "Bad file descriptor")
4347
var statbuf = stat()
4448
var dynamicBuffer: UnsafeMutableRawPointer? = nil
4549
var total = 0
46-
if _closed || fstat(_fd, &statbuf) < 0 {
50+
if fstat(_fd, &statbuf) < 0 {
4751
fatalError("Unable to read file")
4852
}
4953
if statbuf.st_mode & S_IFMT != S_IFREG {
@@ -126,6 +130,7 @@ open class FileHandle : NSObject, NSSecureCoding {
126130
}
127131

128132
open func write(_ data: Data) {
133+
guard _fd >= 0 else { return }
129134
data.enumerateBytes() { (bytes, range, stop) in
130135
do {
131136
try NSData.write(toFileDescriptor: self._fd, path: nil, buf: UnsafeRawPointer(bytes.baseAddress!), length: bytes.count)
@@ -138,39 +143,48 @@ open class FileHandle : NSObject, NSSecureCoding {
138143
// TODO: Error handling.
139144

140145
open var offsetInFile: UInt64 {
146+
precondition(_fd >= 0, "Bad file descriptor")
141147
return UInt64(lseek(_fd, 0, SEEK_CUR))
142148
}
143149

144150
@discardableResult
145151
open func seekToEndOfFile() -> UInt64 {
152+
precondition(_fd >= 0, "Bad file descriptor")
146153
return UInt64(lseek(_fd, 0, SEEK_END))
147154
}
148155

149156
open func seek(toFileOffset offset: UInt64) {
157+
precondition(_fd >= 0, "Bad file descriptor")
150158
lseek(_fd, off_t(offset), SEEK_SET)
151159
}
152160

153161
open func truncateFile(atOffset offset: UInt64) {
162+
precondition(_fd >= 0, "Bad file descriptor")
154163
if lseek(_fd, off_t(offset), SEEK_SET) < 0 { fatalError("lseek() failed.") }
155164
if ftruncate(_fd, off_t(offset)) < 0 { fatalError("ftruncate() failed.") }
156165
}
157166

158167
open func synchronizeFile() {
168+
precondition(_fd >= 0, "Bad file descriptor")
159169
fsync(_fd)
160170
}
161171

162172
open func closeFile() {
163-
if !_closed {
173+
if _fd >= 0 {
164174
close(_fd)
165-
_closed = true
175+
_fd = -1
166176
}
167177
}
168-
178+
169179
public init(fileDescriptor fd: Int32, closeOnDealloc closeopt: Bool) {
170180
_fd = fd
171181
_closeOnDealloc = closeopt
172182
}
173-
183+
184+
public convenience init(fileDescriptor fd: Int32) {
185+
self.init(fileDescriptor: fd, closeOnDealloc: false)
186+
}
187+
174188
internal init?(path: String, flags: Int32, createMode: Int) {
175189
_fd = _CFOpenFileWithMode(path, flags, mode_t(createMode))
176190
_closeOnDealloc = true
@@ -181,8 +195,9 @@ open class FileHandle : NSObject, NSSecureCoding {
181195
}
182196

183197
deinit {
184-
if _fd >= 0 && _closeOnDealloc && !_closed {
198+
if _fd >= 0 && _closeOnDealloc {
185199
close(_fd)
200+
_fd = -1
186201
}
187202
}
188203

@@ -355,38 +370,32 @@ extension FileHandle {
355370
}
356371
}
357372

358-
extension FileHandle {
359-
public convenience init(fileDescriptor fd: Int32) {
360-
self.init(fileDescriptor: fd, closeOnDealloc: false)
361-
}
362-
363-
open var fileDescriptor: Int32 {
364-
return _fd
365-
}
366-
}
367-
368373
open class Pipe: NSObject {
369-
open let fileHandleForReading: FileHandle
370-
open let fileHandleForWriting: FileHandle
374+
public let fileHandleForReading: FileHandle
375+
public let fileHandleForWriting: FileHandle
371376

372377
public override init() {
373378
/// the `pipe` system call creates two `fd` in a malloc'ed area
374379
var fds = UnsafeMutablePointer<Int32>.allocate(capacity: 2)
375380
defer {
376-
free(fds)
381+
fds.deallocate()
377382
}
378383
/// If the operating system prevents us from creating file handles, stop
379-
guard pipe(fds) == 0 else { fatalError("Could not open pipe file handles") }
380-
381-
/// The handles below auto-close when the `NSFileHandle` is deallocated, so we
382-
/// don't need to add a `deinit` to this class
383-
384-
/// Create the read handle from the first fd in `fds`
385-
self.fileHandleForReading = FileHandle(fileDescriptor: fds.pointee, closeOnDealloc: true)
386-
387-
/// Advance `fds` by one to create the write handle from the second fd
388-
self.fileHandleForWriting = FileHandle(fileDescriptor: fds.successor().pointee, closeOnDealloc: true)
389-
384+
let ret = pipe(fds)
385+
switch (ret, errno) {
386+
case (0, _):
387+
self.fileHandleForReading = FileHandle(fileDescriptor: fds.pointee, closeOnDealloc: true)
388+
self.fileHandleForWriting = FileHandle(fileDescriptor: fds.successor().pointee, closeOnDealloc: true)
389+
390+
case (-1, EMFILE), (-1, ENFILE):
391+
// Unfortunately this initializer does not throw and isnt failable so this is only
392+
// way of handling this situation.
393+
self.fileHandleForReading = FileHandle(fileDescriptor: -1, closeOnDealloc: false)
394+
self.fileHandleForWriting = FileHandle(fileDescriptor: -1, closeOnDealloc: false)
395+
396+
default:
397+
fatalError("Error calling pipe(): \(errno)")
398+
}
390399
super.init()
391400
}
392401
}

‎TestFoundation/TestFileHandle.swift‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
11
// This source file is part of the Swift.org open source project
22
//
3-
// Copyright (c) 2016 Apple Inc. and the Swift project authors
3+
// Copyright (c) 2016, 2018 Apple Inc. and the Swift project authors
44
// Licensed under Apache License v2.0 with Runtime Library Exception
55
//
6-
// See http://swift.org/LICENSE.txt for license information
7-
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
6+
// See https://swift.org/LICENSE.txt for license information
7+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
88
//
99

1010
class TestFileHandle : XCTestCase {
1111
static var allTests : [(String, (TestFileHandle) -> () throws -> ())] {
1212
return [
1313
("test_constants", test_constants),
14-
("test_pipe", test_pipe),
1514
("test_nullDevice", test_nullDevice),
1615
("test_truncateFile", test_truncateFile)
1716
]
@@ -22,23 +21,6 @@ class TestFileHandle : XCTestCase {
2221
"\(FileHandle.readCompletionNotification.rawValue) is not equal to NSFileHandleReadCompletionNotification")
2322
}
2423

25-
func test_pipe() {
26-
let pipe = Pipe()
27-
let inputs = ["Hello", "world", "🐶"]
28-
29-
for input in inputs {
30-
let inputData = input.data(using: .utf8)!
31-
32-
// write onto pipe
33-
pipe.fileHandleForWriting.write(inputData)
34-
35-
let outputData = pipe.fileHandleForReading.availableData
36-
let output = String(data: outputData, encoding: .utf8)
37-
38-
XCTAssertEqual(output, input)
39-
}
40-
}
41-
4224
func test_nullDevice() {
4325
let fh = FileHandle.nullDevice
4426

‎TestFoundation/TestPipe.swift‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,41 @@
11
// This source file is part of the Swift.org open source project
22
//
3-
// Copyright (c) 2014 - 2016 Apple Inc. and the Swift project authors
3+
// Copyright (c) 2014 - 2016. 2018 Apple Inc. and the Swift project authors
44
// Licensed under Apache License v2.0 with Runtime Library Exception
55
//
6-
// See http://swift.org/LICENSE.txt for license information
7-
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
6+
// See https://swift.org/LICENSE.txt for license information
7+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
88
//
99

10-
class TestPipe : XCTestCase {
10+
class TestPipe: XCTestCase {
1111

1212
static var allTests: [(String, (TestPipe) -> () throws -> Void)] {
1313
return [
14-
("test_NSPipe", test_NSPipe)
14+
("test_MaxPipes", test_MaxPipes),
15+
("test_Pipe", test_Pipe),
1516
]
1617
}
17-
18-
func test_NSPipe() {
18+
19+
func test_MaxPipes() {
20+
// Try and create enough pipes to exhaust the process's limits. 1024 is a reasonable
21+
// hard limit for the test. This is reached when testing on Linux (at around 488 pipes)
22+
// but not on macOS.
23+
24+
var pipes: [Pipe] = []
25+
let maxPipes = 1024
26+
pipes.reserveCapacity(maxPipes)
27+
for _ in 1...maxPipes {
28+
let pipe = Pipe()
29+
if pipe.fileHandleForReading.fileDescriptor == -1 {
30+
XCTAssertEqual(pipe.fileHandleForReading.fileDescriptor, pipe.fileHandleForWriting.fileDescriptor)
31+
break
32+
}
33+
pipes.append(pipe)
34+
}
35+
pipes = []
36+
}
37+
38+
func test_Pipe() {
1939
let aPipe = Pipe()
2040
let text = "test-pipe"
2141

0 commit comments

Comments
 (0)