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

[ZTS] fclose in mmapwrite.c #14353

Merged

Conversation

aerusso
Copy link
Contributor

@aerusso aerusso commented Jan 6, 2023

Motivation and Context

mmapwrite is used during the ZTS to identify issues with mmap-ed files. This helper program exercises this pathway by continuously writing to a file. ee6bf97 modified the writing threads to terminate after a set amount of total data is written. This change allows standard program execution to reach the end of a writer thread without closing the file descriptor, introducing a resource "leak."

Description

This patch appeases resource leak analyses by fclose()-ing the file at the end of the thread. It also cleans up some error handling.

How Has This Been Tested?

Just the present CI.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@aerusso aerusso force-pushed the mrs/upstream/quiet-coverity-fd-leak branch from 9246e01 to 5a754db Compare January 6, 2023 09:49
mmapwrite is used during the ZTS to identify issues with mmap-ed files.
This helper program exercises this pathway by continuously writing to a
file.  ee6bf97 modified the writing threads to terminate after a set
amount of total data is written.  This change allows standard program
execution to reach the end of a writer thread without closing the file
descriptor, introducing a resource "leak."

This patch appeases resource leak analyses by fclose()-ing the file at
the end of the thread.

Signed-off-by: Antonio Russo <aerusso@aerusso.net>
@aerusso aerusso force-pushed the mrs/upstream/quiet-coverity-fd-leak branch from 5a754db to 2506e81 Compare January 6, 2023 09:59
@behlendorf behlendorf requested a review from ryao January 6, 2023 17:17
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jan 6, 2023
@behlendorf
Copy link
Contributor

Thanks for the quick fix.

Copy link
Contributor

@ryao ryao left a comment

Choose a reason for hiding this comment

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

It should be documented as close. fclose() is for buffered IO. The code itself looks good to me though.

@behlendorf
Copy link
Contributor

I can update the subject line when merging.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jan 6, 2023
@behlendorf behlendorf merged commit a7304ab into openzfs:master Jan 6, 2023
@aerusso aerusso deleted the mrs/upstream/quiet-coverity-fd-leak branch January 7, 2023 02:45
aerusso added a commit to aerusso/zfs that referenced this pull request Jan 7, 2023
commit a7304ab upstream

mmapwrite is used during the ZTS to identify issues with mmap-ed files.
This helper program exercises this pathway by continuously writing to a
file.  ee6bf97 modified the writing threads to terminate after a set
amount of total data is written.  This change allows standard program
execution to reach the end of a writer thread without closing the file
descriptor, introducing a resource "leak."

This patch appeases resource leak analyses by close()-ing the file at
the end of the thread.

Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Antonio Russo <aerusso@aerusso.net>
Closes openzfs#14353
behlendorf pushed a commit that referenced this pull request Jan 10, 2023
commit a7304ab upstream

mmapwrite is used during the ZTS to identify issues with mmap-ed files.
This helper program exercises this pathway by continuously writing to a
file.  ee6bf97 modified the writing threads to terminate after a set
amount of total data is written.  This change allows standard program
execution to reach the end of a writer thread without closing the file
descriptor, introducing a resource "leak."

This patch appeases resource leak analyses by close()-ing the file at
the end of the thread.

Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Antonio Russo <aerusso@aerusso.net>
Closes #14353
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 3, 2023
mmapwrite is used during the ZTS to identify issues with mmap-ed files.
This helper program exercises this pathway by continuously writing to a
file.  ee6bf97 modified the writing threads to terminate after a set
amount of total data is written.  This change allows standard program
execution to reach the end of a writer thread without closing the file
descriptor, introducing a resource "leak."

This patch appeases resource leak analyses by close()-ing the file at
the end of the thread.

Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Antonio Russo <aerusso@aerusso.net>
Closes openzfs#14353
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants