diff --git a/ChangeLog.txt b/ChangeLog.txt index 2b528fd14c..da048a89ca 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -10,6 +10,40 @@ * Change, ! Fix, % Optimization, + Addition, - Removal, ; Comment */ +2015-11-13 10:29 UTC+0100 Przemyslaw Czerpak (druzus/at/poczta.onet.pl) + * src/rtl/filebuf.c + ! fixed file handle sharing in POSIX systems when writeonly mode is used + ! fixed accessing files in writeonly mode in POSIX systems which use + fcntl() locks for DENY_* flag emulation. + ! added for POSIX systems protection against potential race condition in + MT programs and file open and close operations. Now the whole low level + open and close operations are covered by mutex. It means that they are + fully serialized so in case of remote file access (i.e. NFS or SMBFS) + it may reduce the performance in applications which simultaneously open + and closes many different files in different threads but its necessary. + For local file access it should not create noticeable scalability + problems. In my Linux box Harbour can open and close in MT mode more + then 250000 files per second. + % use different mutexes for file open/close and file lock operations to + not reduce the lock performance by potentially slow open() operation + in POSIX systems. + ; NOTE: please remember that DENY_* flag emulation in POSIX systems + emulates only shared and exclusive modes so if any of + FO_DENYREAD, FO_DENYWRITE or FO_EXCLUSIVE flag is used then + it effectively works like FO_EXCLUSIVE in POSIX system and + is not mandatory for other processes so it blocks only + other Harbour applications which use Harbour virtual handles + (RDD code, hb_vf*() functions) + + * src/rtl/filesys.c + * do not convert exclusive lock to shared one when file is open in + readonly mode on POSIX system using flock() for DENY_* flag emulation. + Now HB_USE_BSDLOCKS is enabled only for Linux and it's documented that + this implementation allow to use shared and exclusive locks regardless of + file open mode. + % do not create file name copy in hb_fsExtOpen() when passed parameters + does not force file name modification + 2015-11-05 18:20 UTC+0100 Tamas TEVESZ (ice extreme.hu) * src/3rd/pcre/Makefile ! Fix bundled PCRE build on SunOS; diff --git a/src/rtl/filebuf.c b/src/rtl/filebuf.c index c43bc76402..f91d9523a9 100644 --- a/src/rtl/filebuf.c +++ b/src/rtl/filebuf.c @@ -96,8 +96,8 @@ typedef struct _HB_FILE HB_ULONG device; HB_ULONG inode; int used; + int mode; HB_BOOL shared; - HB_BOOL readonly; HB_FHANDLE hFile; HB_FHANDLE hFileRO; PHB_FLOCK pLocks; @@ -114,6 +114,7 @@ static const HB_FILE_FUNCS * s_fileMethods( void ); #endif static HB_CRITICAL_NEW( s_fileMtx ); +static HB_CRITICAL_NEW( s_lockMtx ); static PHB_FILE s_openFiles = NULL; @@ -150,7 +151,7 @@ static PHB_FILE hb_fileFind( HB_ULONG device, HB_ULONG inode ) return NULL; } -static PHB_FILE hb_fileNew( HB_FHANDLE hFile, HB_BOOL fShared, HB_BOOL fReadonly, +static PHB_FILE hb_fileNew( HB_FHANDLE hFile, HB_BOOL fShared, int iMode, HB_ULONG device, HB_ULONG inode, HB_BOOL fBind ) { PHB_FILE pFile = hb_fileFind( device, inode ); @@ -158,13 +159,13 @@ static PHB_FILE hb_fileNew( HB_FHANDLE hFile, HB_BOOL fShared, HB_BOOL fReadonly if( ! pFile ) { pFile = ( PHB_FILE ) hb_xgrabz( sizeof( HB_FILE ) ); - pFile->pFuncs = s_fileMethods(); - pFile->device = device; - pFile->inode = inode; - pFile->hFile = hFile; - pFile->hFileRO = FS_ERROR; - pFile->shared = fShared; - pFile->readonly = fReadonly; + pFile->pFuncs = s_fileMethods(); + pFile->device = device; + pFile->inode = inode; + pFile->hFile = hFile; + pFile->hFileRO = FS_ERROR; + pFile->shared = fShared; + pFile->mode = iMode; if( fBind ) { @@ -469,27 +470,53 @@ static PHB_FILE s_fileExtOpen( PHB_FILE_FUNCS pFuncs, const char * pszFileName, PHB_ITEM pError ) { PHB_FILE pFile = NULL; - #if defined( HB_OS_UNIX ) - HB_BOOL fResult, fSeek = HB_FALSE; + HB_BOOL fSeek = HB_FALSE; # if defined( HB_USE_LARGEFILE64 ) struct stat64 statbuf; # else struct stat statbuf; # endif #endif - HB_BOOL fShared, fReadonly; + HB_BOOL fResult, fShared; + int iMode; HB_FHANDLE hFile; char * pszFile; HB_SYMBOL_UNUSED( pFuncs ); fShared = ( nExFlags & ( FO_DENYREAD | FO_DENYWRITE | FO_EXCLUSIVE ) ) == 0; - fReadonly = ( nExFlags & ( FO_READ | FO_WRITE | FO_READWRITE ) ) == FO_READ; + iMode = ( int ) ( nExFlags & ( FO_READ | FO_WRITE | FO_READWRITE ) ); pszFile = hb_fsExtName( pszFileName, pDefExt, nExFlags, pPaths ); hb_vmUnlock(); -#if defined( HB_OS_UNIX ) +#if ! defined( HB_OS_UNIX ) + fResult = HB_TRUE; +#else +# if defined( HB_USE_SHARELOCKS ) && ! defined( HB_USE_BSDLOCKS ) + if( nExFlags & FXO_SHARELOCK ) + { + if( iMode == FO_WRITE && fShared ) + { + if( access( ( char * ) pszFile, R_OK ) == 0 || + access( ( char * ) pszFile, F_OK ) != 0 ) + { + nExFlags = ( nExFlags ^ FO_WRITE ) | FO_READWRITE; + iMode = FO_READWRITE; + } + else + nExFlags ^= FXO_SHARELOCK; + } + else if( iMode == FO_READ && ! fShared ) + { + nExFlags &= ~ ( HB_FATTR ) ( FO_DENYREAD | FO_DENYWRITE | FO_EXCLUSIVE ); + fShared = HB_TRUE; + } + } +# endif + + hb_threadEnterCriticalSection( &s_fileMtx ); + # if defined( HB_USE_LARGEFILE64 ) fResult = stat64( ( char * ) pszFile, &statbuf ) == 0; # else @@ -499,57 +526,49 @@ static PHB_FILE s_fileExtOpen( PHB_FILE_FUNCS pFuncs, const char * pszFileName, if( fResult ) { - hb_threadEnterCriticalSection( &s_fileMtx ); - pFile = hb_fileFind( statbuf.st_dev, statbuf.st_ino ); + pFile = hb_fileFind( ( HB_ULONG ) statbuf.st_dev, + ( HB_ULONG ) statbuf.st_ino ); if( pFile ) { if( ! fShared || ! pFile->shared || ( nExFlags & FXO_TRUNCATE ) != 0 ) + { fResult = HB_FALSE; - else if( ! fReadonly && pFile->readonly ) pFile = NULL; - else - pFile->used++; - - if( ( nExFlags & FXO_NOSEEKPOS ) == 0 ) - { -# if defined( HB_OS_VXWORKS ) - fSeek = ! S_ISFIFO( statbuf.st_mode ); -# else - fSeek = ! S_ISFIFO( statbuf.st_mode ) && ! S_ISSOCK( statbuf.st_mode ); -# endif } - } - hb_threadLeaveCriticalSection( &s_fileMtx ); - } - - if( pFile ) - { - if( ! fResult ) - { - hb_fsSetError( ( nExFlags & FXO_TRUNCATE ) ? 5 : 32 ); - pFile = NULL; - } - else if( nExFlags & FXO_COPYNAME ) - hb_strncpy( ( char * ) pszFileName, pszFile, HB_PATH_MAX - 1 ); - - if( pError ) - { - hb_errPutFileName( pError, pszFile ); - if( ! fResult ) + else if( pFile->mode != FO_READWRITE && pFile->mode != iMode ) { - hb_errPutOsCode( pError, hb_fsError() ); - hb_errPutGenCode( pError, ( HB_ERRCODE ) ( ( nExFlags & FXO_TRUNCATE ) ? EG_CREATE : EG_OPEN ) ); + iMode = FO_READWRITE; + pFile = NULL; + } + else + { + pFile->used++; + if( ( nExFlags & FXO_NOSEEKPOS ) == 0 ) + { +# if defined( HB_OS_VXWORKS ) + fSeek = ! S_ISFIFO( statbuf.st_mode ); +# else + fSeek = ! S_ISFIFO( statbuf.st_mode ) && ! S_ISSOCK( statbuf.st_mode ); +# endif + } } } } else -#endif + fResult = HB_TRUE; + + if( fResult && pFile == NULL ) +#endif /* HB_OS_UNIX */ { - hFile = hb_fsExtOpen( pszFileName, pDefExt, nExFlags, pPaths, pError ); + hFile = hb_fsExtOpen( pszFileName, NULL, + nExFlags & ~ ( HB_FATTR ) ( FXO_DEFAULTS | FXO_COPYNAME ), + NULL, NULL ); if( hFile != FS_ERROR ) { HB_ULONG device = 0, inode = 0; -#if defined( HB_OS_UNIX ) +#if ! defined( HB_OS_UNIX ) + hb_threadEnterCriticalSection( &s_fileMtx ); +#else # if defined( HB_USE_LARGEFILE64 ) if( fstat64( hFile, &statbuf ) == 0 ) # else @@ -567,19 +586,30 @@ static PHB_FILE s_fileExtOpen( PHB_FILE_FUNCS pFuncs, const char * pszFileName, # endif } } -#endif +#endif /* HB_OS_UNIX */ - hb_threadEnterCriticalSection( &s_fileMtx ); - pFile = hb_fileNew( hFile, fShared, fReadonly, device, inode, HB_TRUE ); + pFile = hb_fileNew( hFile, fShared, iMode, device, inode, HB_TRUE ); if( pFile->hFile != hFile ) { - if( pFile->hFileRO == FS_ERROR && ! fReadonly && pFile->readonly ) + if( pFile->mode != FO_READWRITE && iMode == FO_READWRITE ) { + HB_FHANDLE hTemp = pFile->hFileRO; pFile->hFileRO = pFile->hFile; pFile->hFile = hFile; - pFile->readonly = HB_FALSE; - hFile = FS_ERROR; + pFile->mode = iMode; + hFile = hTemp; } + + if( ! fShared || ! pFile->shared || pFile->mode != FO_READWRITE ) + { + fResult = HB_FALSE; + if( pFile->hFileRO == FS_ERROR && pFile->uiLocks != 0 ) + { + pFile->hFileRO = hFile; + hFile = FS_ERROR; + } + } + if( pFile->uiLocks == 0 ) { #if ! defined( HB_USE_SHARELOCKS ) || defined( HB_USE_BSDLOCKS ) @@ -595,33 +625,56 @@ static PHB_FILE s_fileExtOpen( PHB_FILE_FUNCS pFuncs, const char * pszFileName, hFile = FS_ERROR; #if defined( HB_USE_SHARELOCKS ) && ! defined( HB_USE_BSDLOCKS ) /* TOFIX: possible race condition */ - hb_fsLockLarge( hFile, HB_SHARELOCK_POS, HB_SHARELOCK_SIZE, + hb_fsLockLarge( pFile->hFile, HB_SHARELOCK_POS, HB_SHARELOCK_SIZE, FL_LOCK | FLX_SHARED ); #endif } } + if( !fResult ) + { + if( pFile ) + { + --pFile->used; + pFile = NULL; + } + if( hFile != FS_ERROR ) + { + /* TOFIX: possible race condition in MT mode, + * close() is not safe due to existing locks + * which are removed. + */ + hb_fsClose( hFile ); + } + } } - else - hFile = FS_ERROR; +#if ! defined( HB_OS_UNIX ) hb_threadLeaveCriticalSection( &s_fileMtx ); - - if( hFile != FS_ERROR ) - { - /* TOFIX: possible race condition in MT mode, - * close() is not safe due to existing locks - * which are removed. - */ - hb_fsClose( hFile ); - } +#endif } } - hb_xfree( pszFile ); #if defined( HB_OS_UNIX ) + hb_threadLeaveCriticalSection( &s_fileMtx ); if( pFile && fSeek ) pFile = hb_fileposNew( pFile ); - #endif + + if( ! fResult ) + hb_fsSetError( ( nExFlags & FXO_TRUNCATE ) ? 5 : 32 ); + if( ( nExFlags & FXO_COPYNAME ) != 0 && pFile ) + hb_strncpy( ( char * ) pszFileName, pszFile, HB_PATH_MAX - 1 ); + if( pError ) + { + hb_errPutFileName( pError, pszFile ); + if( ! fResult ) + { + hb_errPutOsCode( pError, hb_fsError() ); + hb_errPutGenCode( pError, ( HB_ERRCODE ) ( ( nExFlags & FXO_TRUNCATE ) ? EG_CREATE : EG_OPEN ) ); + } + } + + hb_xfree( pszFile ); + hb_vmLock(); return pFile; @@ -629,9 +682,8 @@ static PHB_FILE s_fileExtOpen( PHB_FILE_FUNCS pFuncs, const char * pszFileName, static void s_fileClose( PHB_FILE pFile ) { - HB_FHANDLE hFile = FS_ERROR, hFileRO = FS_ERROR; - hb_vmUnlock(); + hb_fsSetError( 0 ); hb_threadEnterCriticalSection( &s_fileMtx ); if( --pFile->used == 0 ) @@ -647,25 +699,21 @@ static void s_fileClose( PHB_FILE pFile ) s_openFiles = NULL; } } - - hFile = pFile->hFile; - hFileRO = pFile->hFileRO; + if( pFile->hFile != FS_ERROR ) + hb_fsClose( pFile->hFile ); + if( pFile->hFileRO != FS_ERROR ) + hb_fsClose( pFile->hFileRO ); if( pFile->pLocks ) hb_xfree( pFile->pLocks ); hb_xfree( pFile ); } + else + pFile = NULL; hb_threadLeaveCriticalSection( &s_fileMtx ); hb_vmLock(); - - hb_fsSetError( 0 ); - - if( hFile != FS_ERROR ) - hb_fsClose( hFile ); - if( hFileRO != FS_ERROR ) - hb_fsClose( hFileRO ); } static HB_BOOL s_fileLock( PHB_FILE pFile, HB_FOFFSET nStart, HB_FOFFSET nLen, @@ -676,36 +724,38 @@ static HB_BOOL s_fileLock( PHB_FILE pFile, HB_FOFFSET nStart, HB_FOFFSET nLen, hb_vmUnlock(); if( ( iType & FL_MASK ) == FL_UNLOCK ) { - hb_threadEnterCriticalSection( &s_fileMtx ); + hb_threadEnterCriticalSection( &s_lockMtx ); fResult = hb_fileUnlock( pFile, &fLockFS, nStart, nLen ); - hb_threadLeaveCriticalSection( &s_fileMtx ); + hb_threadLeaveCriticalSection( &s_lockMtx ); if( fLockFS ) { hb_fsLockLarge( pFile->hFile, nStart, nLen, ( HB_USHORT ) iType ); - hb_threadEnterCriticalSection( &s_fileMtx ); + hb_threadEnterCriticalSection( &s_lockMtx ); hb_fileUnlock( pFile, NULL, nStart, nLen ); - hb_threadLeaveCriticalSection( &s_fileMtx ); + hb_threadLeaveCriticalSection( &s_lockMtx ); } else hb_fsSetError( fResult ? 0 : 33 ); } else { - hb_threadEnterCriticalSection( &s_fileMtx ); + hb_threadEnterCriticalSection( &s_lockMtx ); fResult = hb_fileSetLock( pFile, &fLockFS, nStart, nLen ); - hb_threadLeaveCriticalSection( &s_fileMtx ); + hb_threadLeaveCriticalSection( &s_lockMtx ); if( fLockFS ) { #if defined( HB_OS_UNIX ) - if( pFile->readonly ) + if( pFile->mode == FO_READ ) iType |= FLX_SHARED; + else if( pFile->mode == FO_WRITE ) + iType &= ~FLX_SHARED; #endif fResult = hb_fsLockLarge( pFile->hFile, nStart, nLen, ( HB_USHORT ) iType ); if( ! fResult ) { - hb_threadEnterCriticalSection( &s_fileMtx ); + hb_threadEnterCriticalSection( &s_lockMtx ); hb_fileUnlock( pFile, NULL, nStart, nLen ); - hb_threadLeaveCriticalSection( &s_fileMtx ); + hb_threadLeaveCriticalSection( &s_lockMtx ); } } else @@ -724,9 +774,9 @@ static int s_fileLockTest( PHB_FILE pFile, HB_FOFFSET nStart, HB_FOFFSET nLen, hb_vmUnlock(); - hb_threadEnterCriticalSection( &s_fileMtx ); + hb_threadEnterCriticalSection( &s_lockMtx ); fLocked = hb_fileTestLock( pFile, nStart, nLen ); - hb_threadLeaveCriticalSection( &s_fileMtx ); + hb_threadLeaveCriticalSection( &s_lockMtx ); if( fLocked ) { #if defined( HB_OS_UNIX ) @@ -1078,7 +1128,7 @@ HB_BOOL hb_fileRegisterFull( const HB_FILE_FUNCS * pFuncs ) HB_BOOL fResult = HB_FALSE; hb_vmUnlock(); - hb_threadEnterCriticalSection( &s_fileMtx ); + hb_threadEnterCriticalSection( &s_lockMtx ); if( s_iFileTypes < HB_FILE_TYPE_MAX ) { @@ -1087,7 +1137,7 @@ HB_BOOL hb_fileRegisterFull( const HB_FILE_FUNCS * pFuncs ) fResult = HB_TRUE; } - hb_threadLeaveCriticalSection( &s_fileMtx ); + hb_threadLeaveCriticalSection( &s_lockMtx ); hb_vmLock(); return fResult; @@ -1491,15 +1541,13 @@ HB_BYTE * hb_fileLoadData( PHB_FILE pFile, HB_SIZE nMaxSize, HB_SIZE * pnSize ) { HB_BYTE * pFileBuf = NULL; - HB_SIZE nSize = 0; + HB_SIZE nSize = 0, nRead, nBufSize; HB_FOFFSET nFileSize = hb_fileSize( pFile ); if( nFileSize == FS_ERROR || ( nFileSize == 0 && hb_fsError() == HB_FILE_ERR_UNSUPPORTED ) ) { - HB_SIZE nRead, nBufSize = 0; - - for( ;; ) + for( nBufSize = 0;; ) { if( nBufSize == nSize ) { @@ -1518,16 +1566,21 @@ HB_BYTE * hb_fileLoadData( PHB_FILE pFile, HB_SIZE nMaxSize, nSize += nRead; } } - else + else if( nFileSize > 0 ) { - nSize = ( HB_SIZE ) nFileSize; - if( nMaxSize > 0 && nSize > nMaxSize ) - nSize = nMaxSize; + nBufSize = ( HB_SIZE ) nFileSize; + if( nMaxSize > 0 && nBufSize > nMaxSize ) + nBufSize = nMaxSize; - pFileBuf = ( HB_BYTE * ) hb_xgrab( nSize + 1 ); - nSize = hb_fileReadAt( pFile, pFileBuf, nSize, 0 ); - if( nSize == ( HB_SIZE ) FS_ERROR ) - nSize = 0; + pFileBuf = ( HB_BYTE * ) hb_xgrab( nBufSize + 1 ); + do + { + nRead = hb_fileReadAt( pFile, pFileBuf + nSize, nBufSize - nSize, nSize ); + if( nRead == 0 || nRead == ( HB_SIZE ) FS_ERROR ) + break; + nSize += nRead; + } + while( nSize < nBufSize ); } if( nSize > 0 ) diff --git a/src/rtl/filesys.c b/src/rtl/filesys.c index 2eae6dbe2a..ccf395ab98 100644 --- a/src/rtl/filesys.c +++ b/src/rtl/filesys.c @@ -4501,7 +4501,8 @@ HB_FHANDLE hb_fsExtOpen( const char * pszFileName, const char * pDefExt, { HB_FHANDLE hFile; HB_USHORT uiFlags; - char * szPath; + const char * szPath; + char * szFree = NULL; HB_TRACE( HB_TR_DEBUG, ( "hb_fsExtOpen(%s, %s, %u, %p, %p)", pszFileName, pDefExt, nExFlags, pPaths, pError ) ); @@ -4520,7 +4521,11 @@ HB_FHANDLE hb_fsExtOpen( const char * pszFileName, const char * pDefExt, hb_errGetFileName( pError ); #endif - szPath = hb_fsExtName( pszFileName, pDefExt, nExFlags, pPaths ); + if( pDefExt || pPaths || pError || + ( nExFlags & ( FXO_DEFAULTS | FXO_COPYNAME ) ) != 0 ) + szPath = szFree = hb_fsExtName( pszFileName, pDefExt, nExFlags, pPaths ); + else + szPath = pszFileName; uiFlags = ( HB_USHORT ) ( nExFlags & 0xff ); if( nExFlags & ( FXO_TRUNCATE | FXO_APPEND | FXO_UNIQUE ) ) @@ -4539,11 +4544,11 @@ HB_FHANDLE hb_fsExtOpen( const char * pszFileName, const char * pDefExt, hFile = hb_fsOpen( szPath, uiFlags ); #if defined( HB_USE_SHARELOCKS ) - if( hFile != FS_ERROR && nExFlags & FXO_SHARELOCK ) + if( hFile != FS_ERROR && ( nExFlags & FXO_SHARELOCK ) != 0 ) { #if defined( HB_USE_BSDLOCKS ) int iLock, iResult; - if( ( uiFlags & ( FO_READ | FO_WRITE | FO_READWRITE ) ) == FO_READ || + if( /* ( uiFlags & ( FO_READ | FO_WRITE | FO_READWRITE ) ) == FO_READ || */ ( uiFlags & ( FO_DENYREAD | FO_DENYWRITE | FO_EXCLUSIVE ) ) == 0 ) iLock = LOCK_SH | LOCK_NB; else @@ -4609,7 +4614,9 @@ HB_FHANDLE hb_fsExtOpen( const char * pszFileName, const char * pDefExt, if( nExFlags & FXO_COPYNAME && hFile != FS_ERROR ) hb_strncpy( ( char * ) pszFileName, szPath, HB_PATH_MAX - 1 ); - hb_xfree( szPath ); + if( szFree ) + hb_xfree( szFree ); + return hFile; }