Skip to content

Fix stdin handling in CUFileDriverScope #5902

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

Merged
merged 2 commits into from
Apr 30, 2025
Merged

Conversation

JanuszL
Copy link
Contributor

@JanuszL JanuszL commented Apr 29, 2025

  • adds stdin backup and restore mechanism to prevent issues with file descriptor
    handling when using cuFile library. This fixes a problem where cuFileDriverOpen/Close
    could close stdin, causing subsequent GDS file operations to fail when the file
    descriptor 0 is reused.

Category:

Bug fix (non-breaking change which fixes an issue)

Description:

  • adds stdin backup and restore mechanism to prevent issues with file descriptor
    handling when using cuFile library. This fixes a problem where cuFileDriverOpen/Close
    could close stdin, causing subsequent GDS file operations to fail when the file
    descriptor 0 is reused.
  • related to nv5021097

Additional information:

Affected modules and functionalities:

  • cufile helper

Key points relevant for the review:

Tests:

  • Existing tests apply
    • test_numpy
    • GDSMem
  • New tests added
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

@JanuszL JanuszL force-pushed the fix_gds_file_desc branch 2 times, most recently from a8b66cc to 76b925e Compare April 29, 2025 15:59
@JanuszL
Copy link
Contributor Author

JanuszL commented Apr 29, 2025

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27700108]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27700108]: BUILD FAILED

@JanuszL JanuszL force-pushed the fix_gds_file_desc branch from 76b925e to e77deac Compare April 29, 2025 17:12
@JanuszL
Copy link
Contributor Author

JanuszL commented Apr 29, 2025

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27704106]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27704106]: BUILD FAILED

- adds stdin backup and restore mechanism to prevent issues with file descriptor
  handling when using cuFile library. This fixes a problem where cuFileDriverOpen
  could close stdin, causing subsequent GDS file operations to fail when the file
  descriptor 0 is reused.

Signed-off-by: Janusz Lisiecki <[email protected]>
@JanuszL JanuszL force-pushed the fix_gds_file_desc branch from e77deac to 1518bfb Compare April 29, 2025 19:30
@JanuszL
Copy link
Contributor Author

JanuszL commented Apr 29, 2025

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27711278]: BUILD STARTED

@@ -32,7 +33,23 @@ namespace test {

struct CUFileDriverScope {
CUFileDriverScope() {
// cuFileDriverOpen is some versions of cuFile library, can close stdin
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// cuFileDriverOpen is some versions of cuFile library, can close stdin
// cuFileDriverOpen in some versions of cuFile library, can close stdin

@@ -43,10 +60,42 @@ struct CUFileDriverScope {
#pragma push_macro("cuFileDriverClose")
#undef cuFileDriverClose
if (cuFileIsSymbolAvailable("cuFileDriverClose_v2")) {
// cuFileDriverOpen is some versions of cuFile library, can close stdin
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// cuFileDriverOpen is some versions of cuFile library, can close stdin
// cuFileDriverOpen in some versions of cuFile library, can close stdin

} else {
// cuFileDriverOpen is some versions of cuFile library, can close stdin
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// cuFileDriverOpen is some versions of cuFile library, can close stdin
// cuFileDriverOpen in some versions of cuFile library, can close stdin

@@ -37,15 +38,47 @@ struct CUFileDriverScope {
CUFileDriverScope() {
// v2 API performs proper reference counting, so we increase the reference count here...
if (cuFileIsSymbolAvailable("cuFileDriverClose_v2")) {
// cuFileDriverOpen is some versions of cuFile library, can close stdin
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// cuFileDriverOpen is some versions of cuFile library, can close stdin
// cuFileDriverOpen in some versions of cuFile library, can close stdin

}
}
~CUFileDriverScope() {
// ...and decrease it here.
// The old GDS API would simply destroy the library, possibly still in use by other modules
// within the process.
if (cuFileIsSymbolAvailable("cuFileDriverClose_v2")) {
// cuFileDriverOpen is some versions of cuFile library, can close stdin
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// cuFileDriverOpen is some versions of cuFile library, can close stdin
// cuFileDriverOpen in some versions of cuFile library, can close stdin

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27711278]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27711278]: BUILD PASSED

Signed-off-by: Janusz Lisiecki <[email protected]>
@JanuszL
Copy link
Contributor Author

JanuszL commented Apr 30, 2025

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27749210]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27749210]: BUILD PASSED

@JanuszL JanuszL merged commit 0684614 into NVIDIA:main Apr 30, 2025
7 checks passed
@JanuszL JanuszL deleted the fix_gds_file_desc branch April 30, 2025 12:08
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.

5 participants