Skip to content

CRL Monitor Test Fix#7138

Merged
dgarske merged 1 commit intowolfSSL:masterfrom
ejohnstown:crl-mon-test-fix
Jan 18, 2024
Merged

CRL Monitor Test Fix#7138
dgarske merged 1 commit intowolfSSL:masterfrom
ejohnstown:crl-mon-test-fix

Conversation

@ejohnstown
Copy link
Copy Markdown
Contributor

@ejohnstown ejohnstown commented Jan 17, 2024

Description

Changed the CRL Monitor test so that it uses the macro STAGE_FILE to stage the CRL files for its test cases. That macro will be copy_file() in most builds. In macOS builds (Mach or FreeBSD), STAGE_FILE will be link_file() which makes a hard link of the CRL file instead of a copy. On the Mac, intermittently, the copy_file() will trigger a CRL monitor update when the file is first opened, not when the copy is complete. link() is atomic.

Testing

Running on a macMini M1, I would get intermittent failures in the testsuite. Just ran it ./configure --enable-all, or ./configure --enable-crl --enable-crl-monitor. I'd just run the testsuite a few times.

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

Copy link
Copy Markdown
Member

@julek-wolfssl julek-wolfssl left a comment

Choose a reason for hiding this comment

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

I like the solution, but I would prefer for it to go into a different function than copy_file. This suggests that the file is really copied while link just creates a hard link which is not a real copy. The tests that currently use copy_file can have macros to use the link version of the function when available.

dgarske
dgarske previously approved these changes Jan 17, 2024
@ejohnstown
Copy link
Copy Markdown
Contributor Author

I like the solution, but I would prefer for it to go into a different function than copy_file. This suggests that the file is really copied while link just creates a hard link which is not a real copy. The tests that currently use copy_file can have macros to use the link version of the function when available.

I think that will introduce more complexity. There's a note in copy_file that it is using link() in place of a copy.

Comment thread src/crl.c Outdated
@julek-wolfssl
Copy link
Copy Markdown
Member

I think that will introduce more complexity. There's a note in copy_file that it is using link() in place of a copy.

Then change the file name to something more relevant. Like copy_const_file with documentation at the top of the function that the output file can not be modified.

@julek-wolfssl
Copy link
Copy Markdown
Member

I would get intermittent failures in the testsuite

Is the 100 ms delay too short? Would increasing that make it stop failing? Increasing the delay would simplify this PR.

@ejohnstown
Copy link
Copy Markdown
Contributor Author

I would get intermittent failures in the testsuite

Is the 100 ms delay too short? Would increasing that make it stop failing? Increasing the delay would simplify this PR.

I've bumped it up to 1 second and it still had issues.

@julek-wolfssl
Copy link
Copy Markdown
Member

I've bumped it up to 1 second and it still had issues.

I don't believe that the mac is that slow. Did you make sure to modify the last sleep not the others?

@ejohnstown ejohnstown force-pushed the crl-mon-test-fix branch 3 times, most recently from 3fd9b52 to 079838f Compare January 18, 2024 02:26
1. For Mach and FreeBsd builds, add the function link_file() which makes
   a hard link for a file.
2. Add a macro STAGE_FILE that either calls copy_file or link_file
   depending on doing a Mach or FreeBSD build or not.

This is to work around how the CRL Monitor is detecting file changes
made by the CRL monitor test in the testsuite. Linux and Windows are
detecting the file copies and deletes, and how macOS detects them.
kevent sees the link as a single change to the parent directory and
reads it. When you copy the file, kevent sees the new file getting
opened and triggering the file update.
@dgarske dgarske merged commit ec96fcd into wolfSSL:master Jan 18, 2024
@ejohnstown ejohnstown deleted the crl-mon-test-fix branch January 18, 2024 16:29
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.

4 participants