Skip to content

Commit e691868

Browse files
weissiweissi
authored
Merge pull request #1893 from spevans/pr_sr_7608_42
[4.2] SR-7608: Pipe() crashes when the underlying system call fails.
2 parents dc6ecd6 + f84dece commit e691868

File tree

3 files changed

+77
-74
lines changed

3 files changed

+77
-74
lines changed

‎Foundation/FileHandle.swift

Lines changed: 47 additions & 46 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,10 +16,13 @@ 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
22-
19+
private var _fd: Int32
20+
private var _closeOnDealloc: Bool
21+
22+
open var fileDescriptor: Int32 {
23+
return _fd
24+
}
25+
2326
open var availableData: Data {
2427
return _readDataOfLength(Int.max, untilEOF: false)
2528
}
@@ -33,10 +36,11 @@ open class FileHandle : NSObject, NSSecureCoding {
3336
}
3437

3538
internal func _readDataOfLength(_ length: Int, untilEOF: Bool) -> Data {
39+
precondition(_fd >= 0, "Bad file descriptor")
3640
var statbuf = stat()
3741
var dynamicBuffer: UnsafeMutableRawPointer? = nil
3842
var total = 0
39-
if _closed || fstat(_fd, &statbuf) < 0 {
43+
if fstat(_fd, &statbuf) < 0 {
4044
fatalError("Unable to read file")
4145
}
4246
if statbuf.st_mode & S_IFMT != S_IFREG {
@@ -119,6 +123,7 @@ open class FileHandle : NSObject, NSSecureCoding {
119123
}
120124

121125
open func write(_ data: Data) {
126+
precondition(_fd >= 0, "Bad file descriptor")
122127
data.enumerateBytes() { (bytes, range, stop) in
123128
do {
124129
try NSData.write(toFileDescriptor: self._fd, path: nil, buf: UnsafeRawPointer(bytes.baseAddress!), length: bytes.count)
@@ -131,40 +136,49 @@ open class FileHandle : NSObject, NSSecureCoding {
131136
// TODO: Error handling.
132137

133138
open var offsetInFile: UInt64 {
139+
precondition(_fd >= 0, "Bad file descriptor")
134140
return UInt64(lseek(_fd, 0, SEEK_CUR))
135141
}
136142

137143
@discardableResult
138144
open func seekToEndOfFile() -> UInt64 {
145+
precondition(_fd >= 0, "Bad file descriptor")
139146
return UInt64(lseek(_fd, 0, SEEK_END))
140147
}
141148

142149
open func seek(toFileOffset offset: UInt64) {
150+
precondition(_fd >= 0, "Bad file descriptor")
143151
lseek(_fd, off_t(offset), SEEK_SET)
144152
}
145153

146154
open func truncateFile(atOffset offset: UInt64) {
155+
precondition(_fd >= 0, "Bad file descriptor")
147156
if lseek(_fd, off_t(offset), SEEK_SET) == 0 {
148157
ftruncate(_fd, off_t(offset))
149158
}
150159
}
151160

152161
open func synchronizeFile() {
162+
precondition(_fd >= 0, "Bad file descriptor")
153163
fsync(_fd)
154164
}
155165

156166
open func closeFile() {
157-
if !_closed {
167+
if _fd >= 0 {
158168
close(_fd)
159-
_closed = true
169+
_fd = -1
160170
}
161171
}
162-
172+
163173
public init(fileDescriptor fd: Int32, closeOnDealloc closeopt: Bool) {
164174
_fd = fd
165175
_closeOnDealloc = closeopt
166176
}
167-
177+
178+
public convenience init(fileDescriptor fd: Int32) {
179+
self.init(fileDescriptor: fd, closeOnDealloc: false)
180+
}
181+
168182
internal init?(path: String, flags: Int32, createMode: Int) {
169183
_fd = _CFOpenFileWithMode(path, flags, mode_t(createMode))
170184
_closeOnDealloc = true
@@ -175,8 +189,9 @@ open class FileHandle : NSObject, NSSecureCoding {
175189
}
176190

177191
deinit {
178-
if _fd >= 0 && _closeOnDealloc && !_closed {
192+
if _fd >= 0 && _closeOnDealloc {
179193
close(_fd)
194+
_fd = -1
180195
}
181196
}
182197

@@ -357,46 +372,32 @@ extension FileHandle {
357372
}
358373
}
359374

360-
extension FileHandle {
361-
public convenience init(fileDescriptor fd: Int32) {
362-
self.init(fileDescriptor: fd, closeOnDealloc: false)
363-
}
364-
365-
open var fileDescriptor: Int32 {
366-
return _fd
367-
}
368-
}
369-
370375
open class Pipe: NSObject {
371-
private let readHandle: FileHandle
372-
private let writeHandle: FileHandle
373-
376+
public let fileHandleForReading: FileHandle
377+
public let fileHandleForWriting: FileHandle
378+
374379
public override init() {
375380
/// the `pipe` system call creates two `fd` in a malloc'ed area
376381
var fds = UnsafeMutablePointer<Int32>.allocate(capacity: 2)
377382
defer {
378-
free(fds)
383+
fds.deallocate()
379384
}
380385
/// If the operating system prevents us from creating file handles, stop
381-
guard pipe(fds) == 0 else { fatalError("Could not open pipe file handles") }
382-
383-
/// The handles below auto-close when the `NSFileHandle` is deallocated, so we
384-
/// don't need to add a `deinit` to this class
385-
386-
/// Create the read handle from the first fd in `fds`
387-
self.readHandle = FileHandle(fileDescriptor: fds.pointee, closeOnDealloc: true)
388-
389-
/// Advance `fds` by one to create the write handle from the second fd
390-
self.writeHandle = FileHandle(fileDescriptor: fds.successor().pointee, closeOnDealloc: true)
391-
386+
let ret = pipe(fds)
387+
switch (ret, errno) {
388+
case (0, _):
389+
self.fileHandleForReading = FileHandle(fileDescriptor: fds.pointee, closeOnDealloc: true)
390+
self.fileHandleForWriting = FileHandle(fileDescriptor: fds.successor().pointee, closeOnDealloc: true)
391+
392+
case (-1, EMFILE), (-1, ENFILE):
393+
// Unfortunately this initializer does not throw and isnt failable so this is only
394+
// way of handling this situation.
395+
self.fileHandleForReading = FileHandle(fileDescriptor: -1, closeOnDealloc: false)
396+
self.fileHandleForWriting = FileHandle(fileDescriptor: -1, closeOnDealloc: false)
397+
398+
default:
399+
fatalError("Error calling pipe(): \(errno)")
400+
}
392401
super.init()
393402
}
394-
395-
open var fileHandleForReading: FileHandle {
396-
return self.readHandle
397-
}
398-
399-
open var fileHandleForWriting: FileHandle {
400-
return self.writeHandle
401-
}
402403
}

‎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
]
1716
}
@@ -21,23 +20,6 @@ class TestFileHandle : XCTestCase {
2120
"\(FileHandle.readCompletionNotification.rawValue) is not equal to NSFileHandleReadCompletionNotification")
2221
}
2322

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

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