Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SR-7608: Pipe() crashes when the underlying system call fails. #1697

Merged
merged 1 commit into from Oct 10, 2018

Conversation

spevans
Copy link
Collaborator

@spevans spevans commented Sep 16, 2018

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

- 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.
@spevans
Copy link
Collaborator Author

spevans commented Sep 16, 2018

@swift-ci test

3 similar comments
@spevans
Copy link
Collaborator Author

spevans commented Sep 17, 2018

@swift-ci test

@spevans
Copy link
Collaborator Author

spevans commented Sep 17, 2018

@swift-ci test

@spevans
Copy link
Collaborator Author

spevans commented Sep 18, 2018

@swift-ci test

@parkera
Copy link
Member

parkera commented Sep 18, 2018

Do we need to provide some new API on Darwin that avoids throwing an ObjC exception?

@parkera parkera self-requested a review September 18, 2018 21:59
@spevans
Copy link
Collaborator Author

spevans commented Sep 19, 2018

On Darwin Pipe() doesn't actually throw an exception but erroneously sets the pipe ends to be 0.

eg:


var pipes: [Pipe] = []
while true {
    let pipe = Pipe()
    print (pipe.fileHandleForReading.fileDescriptor, pipe.fileHandleForWriting.fileDescriptor)
    if pipe.fileHandleForReading.fileDescriptor == 0 {
	print("Created \(pipes.count) pipes")
	break
    }
    pipes.append(pipe)
}

Will create about 1266 pipes on 10.13.6 before returning 0 for the file descriptors. Maybe this initialiser should be deprecated and a failable one introduced?

@parkera
Copy link
Member

parkera commented Sep 19, 2018

Interesting. I'll track that with a radar for us. rdar://problem/44605663

// hard limit for the test. This is reached when testing on Linux (at around 488 pipes)
// but not on macOS.

var pipes: [Pipe] = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried about this, because it means that the test process cannot use pipes meaningfully anywhere else.

Can we move this code to a separate process we spawn?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the pipes should be closed when the array is cleared. test_MaxPipes is run before test_Pipe to test this very case.

@spevans
Copy link
Collaborator Author

spevans commented Sep 26, 2018

@swift-ci test

@millenomi
Copy link
Contributor

@swift-ci please test and merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants