Skip to content

gh-102765: Updated isdir/isfile/islink/exists to use Py_GetFileInformationByName in ntpath when available #103485

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.

Sign up for GitHub

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
zooba merged 27 commits into from
Apr 27, 2023
+209 −118
Merged

gh-102765: Updated isdir/isfile/islink/exists to use Py_GetFileInformationByName in ntpath when available #103485

Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b9c4dbe
pythongh-102765: added _Py_GetFileInformationByName to isdir
finnagin Mar 27, 2023
9faeafa
Merge branch 'python:main' into gh-102765
finnagin Mar 27, 2023
a136922
enable actions workflows for branch
finnagin Mar 27, 2023
a1a3fa5
pythongh-102765: add missing curly brace, fix whitespace
finnagin Mar 27, 2023
aa984f2
pythongh-102765: added _Py_GetFileInformationByName to exists & isfile
finnagin Mar 27, 2023
fb95b7f
pythongh-102765: added _Py_GetFileInformationByName to islink, altere…
finnagin Mar 28, 2023
83de4de
fix whitespace, remove unneeded comments
finnagin Mar 28, 2023
1b47b63
add results
finnagin Mar 28, 2023
43e203d
added result for slow_path excluding errors
finnagin Mar 28, 2023
0d373d1
skip isdir/isfile slow path if not a dir/file
finnagin Mar 28, 2023
82cab87
Merge branch 'python:main' into gh-102765
finnagin Mar 28, 2023
25fbf06
Merge branch 'python:main' into gh-102765
finnagin Mar 29, 2023
4278113
remove extra close_file
finnagin Mar 29, 2023
c3b9d32
merge python:main into 102765
finnagin Mar 29, 2023
9b2544d
merge python:main into 102765
finnagin Mar 30, 2023
9be05ae
merge python:main into 102765
finnagin Mar 30, 2023
97dc17d
merge python:main into 102765
finnagin Mar 31, 2023
72ed317
change path -> _path.wide
finnagin Apr 3, 2023
0e6bd58
merge python:main into 102765
finnagin Apr 3, 2023
a78f697
merge python:main into 102765
finnagin Apr 3, 2023
36e8a9d
merge python:main into 102765
finnagin Apr 11, 2023
b1593af
merge python:main into 102765
finnagin Apr 12, 2023
94f8eeb
revert branch addition in actions workflow
finnagin Apr 12, 2023
86b323d
merge python:main into 102765
finnagin Apr 13, 2023
5aa3e8a
Excluded additional errors from slow fallback path
finnagin Apr 13, 2023
3338503
Disable slow path in islink on any successful GetFileInformationByNam…
finnagin Apr 13, 2023
9eb3e37
move error switch into static inline function
finnagin Apr 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
gh-102765: added _Py_GetFileInformationByName to islink, altered slow… 
…_path logic
  • Loading branch information
@finnagin
finnagin committed Mar 28, 2023
commit fb95b7fd4db367e4795919e753cbea1d3d86a489
276 changes: 155 additions & 121 deletions Modules/posixmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4824,43 +4824,45 @@ os__path_isdir_impl(PyObject *module, PyObject *path)
}
}
}
if (_path.fd != -1) {
hfile = _Py_get_osfhandle_noraise(_path.fd);
close_file = FALSE;
}
else if(slow_path){
hfile = CreateFileW(_path.wide, FILE_READ_ATTRIBUTES, 0, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
}
if (slow_path && hfile != INVALID_HANDLE_VALUE) {
if (GetFileInformationByHandleEx(hfile, FileBasicInfo, &info,
sizeof(info)))
{
result = info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY;
if (slow_path){
if (_path.fd != -1) {
hfile = _Py_get_osfhandle_noraise(_path.fd);
close_file = FALSE;
Comment on lines +4821 to +4823
Copy link
Copy Markdown
Contributor

@eryksun eryksun Apr 13, 2023

Choose a reason for hiding this comment

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

A couple of bugs slipped by when the new accelerator functions were added. These bugs should probably be resolved in a separate issue before this pull request gets merged.

  • If _Py_get_osfhandle_noraise() fails (e.g. a bad file descriptor), the result is INVALID_HANDLE_VALUE, but the thread's last error isn't set and is thus a random error code. It could be one of the errors that's handled by calling STAT(), but in this case there is no _path.wide value to check. It happens that CreateFileW() doesn't raise an OS exception that crashes the process when passed a null pointer for lpFileName, but that's not documented and shouldn't be relied on.
  • If GetFileType(hfile) isn't FILE_TYPE_DISK, we have to return a false result. Since we didn't open the file, we don't know whether or not it has a pending synchronous operation that's blocked indefinitely. For example, a pipe or character file could have a pending synchronous read that may never complete. In this case, GetFileInformationByHandleEx() would block.

}
else {
result = 0;
}
if (close_file) {
CloseHandle(hfile);
hfile = CreateFileW(_path.wide, FILE_READ_ATTRIBUTES, 0, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
}
}
else {
STRUCT_STAT st;
switch (GetLastError()) {
case ERROR_ACCESS_DENIED:
case ERROR_SHARING_VIOLATION:
case ERROR_CANT_ACCESS_FILE:
case ERROR_INVALID_PARAMETER:
if (STAT(_path.wide, &st)) {
result = 0;
if (hfile != INVALID_HANDLE_VALUE) {
if (GetFileInformationByHandleEx(hfile, FileBasicInfo, &info,
sizeof(info)))
{
result = info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY;
}
else {
result = S_ISDIR(st.st_mode);
result = 0;
}
if (close_file) {
CloseHandle(hfile);
}
}
else {
STRUCT_STAT st;
switch (GetLastError()) {
case ERROR_ACCESS_DENIED:
case ERROR_SHARING_VIOLATION:
case ERROR_CANT_ACCESS_FILE:
case ERROR_INVALID_PARAMETER:
if (STAT(_path.wide, &st)) {
result = 0;
}
else {
result = S_ISDIR(st.st_mode);
}
break;
default:
result = 0;
}
break;
default:
result = 0;
}
}
Py_END_ALLOW_THREADS
Expand Down Expand Up @@ -4926,43 +4928,45 @@ os__path_isfile_impl(PyObject *module, PyObject *path)
}
}
}
if (_path.fd != -1) {
hfile = _Py_get_osfhandle_noraise(_path.fd);
close_file = FALSE;
}
else if(slow_path){
hfile = CreateFileW(_path.wide, FILE_READ_ATTRIBUTES, 0, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
}
if (slow_path && hfile != INVALID_HANDLE_VALUE) {
if (GetFileInformationByHandleEx(hfile, FileBasicInfo, &info,
sizeof(info)))
{
result = !(info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY);
if (slow_path){
if (_path.fd != -1) {
hfile = _Py_get_osfhandle_noraise(_path.fd);
close_file = FALSE;
}
else {
result = 0;
hfile = CreateFileW(_path.wide, FILE_READ_ATTRIBUTES, 0, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
}
if (close_file) {
CloseHandle(hfile);
}
}
else {
STRUCT_STAT st;
switch (GetLastError()) {
case ERROR_ACCESS_DENIED:
case ERROR_SHARING_VIOLATION:
case ERROR_CANT_ACCESS_FILE:
case ERROR_INVALID_PARAMETER:
if (STAT(_path.wide, &st)) {
result = 0;
if (hfile != INVALID_HANDLE_VALUE) {
if (GetFileInformationByHandleEx(hfile, FileBasicInfo, &info,
sizeof(info)))
{
result = !(info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY);
}
else {
result = S_ISREG(st.st_mode);
result = 0;
}
if (close_file) {
CloseHandle(hfile);
}
}
else {
STRUCT_STAT st;
switch (GetLastError()) {
case ERROR_ACCESS_DENIED:
case ERROR_SHARING_VIOLATION:
case ERROR_CANT_ACCESS_FILE:
case ERROR_INVALID_PARAMETER:
if (STAT(_path.wide, &st)) {
result = 0;
}
else {
result = S_ISREG(st.st_mode);
}
break;
default:
result = 0;
}
break;
default:
result = 0;
}
}
Py_END_ALLOW_THREADS
Expand Down Expand Up @@ -5027,36 +5031,38 @@ os__path_exists_impl(PyObject *module, PyObject *path)
}
}
}
if (_path.fd != -1) {
hfile = _Py_get_osfhandle_noraise(_path.fd);
close_file = FALSE;
}
else if(slow_path){
hfile = CreateFileW(_path.wide, FILE_READ_ATTRIBUTES, 0, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
}
if (slow_path && hfile != INVALID_HANDLE_VALUE) {
result = 1;
if (close_file) {
CloseHandle(hfile);
if (slow_path){
if (_path.fd != -1) {
hfile = _Py_get_osfhandle_noraise(_path.fd);
close_file = FALSE;
}
}
else {
STRUCT_STAT st;
switch (GetLastError()) {
case ERROR_ACCESS_DENIED:
case ERROR_SHARING_VIOLATION:
case ERROR_CANT_ACCESS_FILE:
case ERROR_INVALID_PARAMETER:
if (STAT(_path.wide, &st)) {
result = 0;
else {
hfile = CreateFileW(_path.wide, FILE_READ_ATTRIBUTES, 0, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
}
if (hfile != INVALID_HANDLE_VALUE) {
result = 1;
if (close_file) {
CloseHandle(hfile);
}
else {
result = 1;
}
else {
STRUCT_STAT st;
switch (GetLastError()) {
case ERROR_ACCESS_DENIED:
case ERROR_SHARING_VIOLATION:
case ERROR_CANT_ACCESS_FILE:
case ERROR_INVALID_PARAMETER:
if (STAT(_path.wide, &st)) {
result = 0;
}
else {
result = 1;
}
break;
default:
result = 0;
}
break;
default:
result = 0;
}
}
Py_END_ALLOW_THREADS
Expand Down Expand Up @@ -5087,6 +5093,8 @@ os__path_islink_impl(PyObject *module, PyObject *path)
FILE_ATTRIBUTE_TAG_INFO info;
path_t _path = PATH_T_INITIALIZE("islink", "path", 0, 1);
int result;
BOOL slow_path = TRUE;
FILE_STAT_BASIC_INFORMATION statInfo;

if (!path_converter(path, &_path)) {
path_cleanup(&_path);
Expand All @@ -5098,45 +5106,71 @@ os__path_islink_impl(PyObject *module, PyObject *path)
}

Py_BEGIN_ALLOW_THREADS
if (_path.fd != -1) {
hfile = _Py_get_osfhandle_noraise(_path.fd);
close_file = FALSE;
if(_path.wide){
if (_Py_GetFileInformationByName(path, FileStatBasicByNameInfo,
&statInfo, sizeof(statInfo))) {
if (// Cannot use fast path for reparse points ...
!(statInfo.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)
// ... unless it's a name surrogate (symlink) and we're not following
|| IsReparseTagNameSurrogate(statInfo.ReparseTag)
) {
slow_path = FALSE;
}
} else {
switch(GetLastError()) {
case ERROR_FILE_NOT_FOUND:
case ERROR_PATH_NOT_FOUND:
case ERROR_NOT_READY:
case ERROR_BAD_NET_NAME:
/* These errors aren't worth retrying with the slow path */
slow_path = FALSE;
case ERROR_NOT_SUPPORTED:
/* indicates the API couldn't be loaded */
break;
}
}
}
else {
hfile = CreateFileW(_path.wide, FILE_READ_ATTRIBUTES, 0, NULL,
OPEN_EXISTING,
FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS,
NULL);
}
if (hfile != INVALID_HANDLE_VALUE) {
if (GetFileInformationByHandleEx(hfile, FileAttributeTagInfo, &info,
sizeof(info)))
{
result = (info.ReparseTag == IO_REPARSE_TAG_SYMLINK);
if (slow_path){
if (_path.fd != -1) {
hfile = _Py_get_osfhandle_noraise(_path.fd);
close_file = FALSE;
}
else {
result = 0;
}
if (close_file) {
CloseHandle(hfile);
hfile = CreateFileW(_path.wide, FILE_READ_ATTRIBUTES, 0, NULL,
OPEN_EXISTING,
FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS,
NULL);
}
}
else {
STRUCT_STAT st;
switch (GetLastError()) {
case ERROR_ACCESS_DENIED:
case ERROR_SHARING_VIOLATION:
case ERROR_CANT_ACCESS_FILE:
case ERROR_INVALID_PARAMETER:
if (LSTAT(_path.wide, &st)) {
result = 0;
if (hfile != INVALID_HANDLE_VALUE) {
if (GetFileInformationByHandleEx(hfile, FileAttributeTagInfo, &info,
sizeof(info)))
{
result = (info.ReparseTag == IO_REPARSE_TAG_SYMLINK);
}
else {
result = S_ISLNK(st.st_mode);
result = 0;
}
if (close_file) {
CloseHandle(hfile);
}
}
else {
STRUCT_STAT st;
switch (GetLastError()) {
case ERROR_ACCESS_DENIED:
case ERROR_SHARING_VIOLATION:
case ERROR_CANT_ACCESS_FILE:
case ERROR_INVALID_PARAMETER:
if (LSTAT(_path.wide, &st)) {
result = 0;
}
else {
result = S_ISLNK(st.st_mode);
}
break;
default:
result = 0;
}
break;
default:
result = 0;
}
}
Py_END_ALLOW_THREADS
Expand Down