Nothing Special   »   [go: up one dir, main page]

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

Fix bug #66150: SOAP WSDL cache race condition causes Segmentation Fault #12469

Closed
wants to merge 2 commits into from

Conversation

nielsdos
Copy link
Member

When we have two processes both trying to cache a WSDL, they might start writing the data to the same temporary file, causing file corruption due to the race condition. Fix this by creating a temporary file first, and then moving it to the final location. If moving fails then we know another process finished caching first.

This also fixes #67617 as a consequence of its implementation.

When we have two processes both trying to cache a WSDL, they might start
writing the data to the same temporary file, causing file corruption due
to the race condition. Fix this by creating a temporary file first, and
then moving it to the final location. If moving fails then we know
another process finished caching first.

This also fixes #67617 as a consequence of its implementation.
/* To avoid race conditions, we first create a temporary file and then rename it atomically
* at the end of the function. (see bug #66150) */
zend_string *temp_file_path;
f = php_open_temporary_fd_ex(NULL, "tmp.wsdl.", &temp_file_path, PHP_TMP_FILE_SILENT);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if rename() may work for different volumes. It's better to create temporary file at the same directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK I changed first argument so that cache_dir is used, now they are in same directory.

@nielsdos nielsdos closed this in 43e6316 Oct 19, 2023
@nielsdos
Copy link
Member Author
nielsdos commented Oct 19, 2023

Thanks for review.

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

Successfully merging this pull request may close these issues.

2 participants