tinyxml2 icon indicating copy to clipboard operation
tinyxml2 copied to clipboard

fopen_s on Windows doesn't allow for concurrent read-only access of files

Open andres-a-salas opened this issue 3 years ago • 0 comments

What's happening:

On Windows, and for specific versions of MSVC, the following code is used to open files: (see lines 2290-2297 in tinyxml2.cpp):

#if defined(_MSC_VER) && (_MSC_VER >= 1400 ) && (!defined WINCE)
    
FILE* fp = 0;
    
const errno_t err = fopen_s( &fp, filepath, mode );
    if ( err ) {
        return 0;
    }
#else
    FILE* fp = fopen( filepath, mode );

It would seem that this code was added to avoid a warning using MSVC, as using fopen does indeed result in a warning on MSVC14, with the warning message suggesting to use fopen_s instead.

According to the Microsoft API entry for fopen_s, however, (see the remarks section in https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-s-wfopen-s?view=msvc-170), fopen_s does not open files while allowing for file sharing.

This means that a multithreaded or multiprocess program opening a file using tinyxml will occasionally fail to obtain file access if another thread or process currently has that file open, even in read-only mode.

To avoid this issue, and also compile without a warning, I have used Microsoft's recommended alternative for file sharing, _fsopen for the Windows only case (this requres including Share.h at the beginning of the file, using the same #define that enables this code to be ran):

 FILE* fp = 0;
 fp = _fsopen(filename, mode, _SH_DENYNO);
 if (fp)
     return fp;
  return 0;

The downside to this function is that it does not return a descriptive error code as fopen_s does, however the error code wasn't used outside of the scope of this function, so perhaps that is alright, perhaps not. I have not observed any issues while using this alternative, but of course I may have missed one or more.

andres-a-salas avatar Jul 07 '22 19:07 andres-a-salas