diff --git a/harbour/ChangeLog b/harbour/ChangeLog index 8599df69ed..8dc9f00bef 100644 --- a/harbour/ChangeLog +++ b/harbour/ChangeLog @@ -16,6 +16,29 @@ The license applies to all entries newer than 2009-04-28. */ +2011-03-19 06:59 UTC+0100 Przemyslaw Czerpak (druzus/at/priv.onet.pl) + * harbour/src/rtl/filesys.c + ! added protection for potentially unfinished due to signal + interrupt close() operation in *nix systems in hb_fsClose() + function + * use hb_fsClose() instead of close() to be sure that the operation + is completed in *nix systems + * return NULL for DosToWinHandle( FS_ERROR ) + ; TODO: add protection against signal interrupting to all hb_fs*() + functions which may need it. + + * harbour/src/rtl/hbproces.c + * use hb_fsClose() instead of close() to be sure that the operation + is completed in *nix systems + ! Do not inherit ends of pipe handles in MS-Windows builds of + hb_fsProcessOpen() + ! Added missing CloseHandle( hProcess ) in MS-Windows builds of + hb_fsProcessRun() + ; QUESTION: Does hb_fsProcessRun() work correctly in MS-Windows builds? + I've just noticed that I used WaitForMultipleObjects() + WIN API function with unnamed pipe handles and I cannot + find any information in MSDN that this is legal operation. + 2011-03-18 15:34 UTC-0800 Pritpal Bedi (bedipritpal@hotmail.com) * contrib/hbqt/qtgui/qth/QActionEvent.qth * contrib/hbqt/qtgui/qth/QContextMenuEvent.qth diff --git a/harbour/src/rtl/filesys.c b/harbour/src/rtl/filesys.c index 18c7a60f5e..dcac2de130 100644 --- a/harbour/src/rtl/filesys.c +++ b/harbour/src/rtl/filesys.c @@ -120,6 +120,9 @@ #include #endif #endif +#if !defined( HB_OS_WIN ) +# include +#endif #if ( defined( __DMC__ ) || defined( __BORLANDC__ ) || \ defined( __IBMCPP__ ) || defined( _MSC_VER ) || \ @@ -369,7 +372,10 @@ static void fs_win_set_drive( int iDrive ) static HANDLE DosToWinHandle( HB_FHANDLE fHandle ) { - if( fHandle == ( HB_FHANDLE ) HB_STDIN_HANDLE ) + if( fHandle == ( HB_FHANDLE ) FS_ERROR ) + return NULL; + + else if( fHandle == ( HB_FHANDLE ) HB_STDIN_HANDLE ) return GetStdHandle( STD_INPUT_HANDLE ); else if( fHandle == ( HB_FHANDLE ) HB_STDOUT_HANDLE ) @@ -654,12 +660,12 @@ HB_FHANDLE hb_fsPOpen( const char * pFilename, const char * pMode ) { if( fRead ) { - close( hPipeHandle[ 1 ] ); + hb_fsClose( hPipeHandle[ 1 ] ); hFileHandle = hPipeHandle[ 0 ]; } else { - close( hPipeHandle[ 0 ] ); + hb_fsClose( hPipeHandle[ 0 ] ); hFileHandle = hPipeHandle[ 1 ]; } } @@ -673,14 +679,14 @@ HB_FHANDLE hb_fsPOpen( const char * pFilename, const char * pMode ) hNullHandle = open( "/dev/null", O_RDWR ); if( fRead ) { - close( hPipeHandle[ 0 ] ); + hb_fsClose( hPipeHandle[ 0 ] ); dup2( hPipeHandle[ 1 ], 1 ); dup2( hNullHandle, 0 ); dup2( hNullHandle, 2 ); } else { - close( hPipeHandle[ 1 ] ); + hb_fsClose( hPipeHandle[ 1 ] ); dup2( hPipeHandle[ 0 ], 0 ); dup2( hNullHandle, 1 ); dup2( hNullHandle, 2 ); @@ -689,7 +695,7 @@ HB_FHANDLE hb_fsPOpen( const char * pFilename, const char * pMode ) if( iMaxFD < 3 ) iMaxFD = 1024; for( hNullHandle = 3; hNullHandle < iMaxFD; ++hNullHandle ) - close( hNullHandle ); + hb_fsClose( hNullHandle ); setuid( getuid() ); setgid( getgid() ); #if defined( __WATCOMC__ ) @@ -702,8 +708,8 @@ HB_FHANDLE hb_fsPOpen( const char * pFilename, const char * pMode ) } else { - close( hPipeHandle[ 0 ] ); - close( hPipeHandle[ 1 ] ); + hb_fsClose( hPipeHandle[ 0 ] ); + hb_fsClose( hPipeHandle[ 1 ] ); } } hb_fsSetIOError( hFileHandle != FS_ERROR, 0 ); @@ -898,7 +904,21 @@ void hb_fsClose( HB_FHANDLE hFileHandle ) #if defined( HB_OS_WIN ) hb_fsSetIOError( CloseHandle( DosToWinHandle( hFileHandle ) ) != 0, 0 ); #else - hb_fsSetIOError( close( hFileHandle ) == 0, 0 ); + { + int ret; +# if defined( EINTR ) + /* ignoring EINTR in close() it's quite common bug when sockets or + * pipes are used. Without such protection it's not safe to use + * signals in user code. + */ + do + ret = close( hFileHandle ); + while( ret == -1 && errno == EINTR ); +# else + ret = close( hFileHandle ); +# endif + hb_fsSetIOError( ret == 0, 0 ); + } #endif hb_vmLock(); } diff --git a/harbour/src/rtl/hbproces.c b/harbour/src/rtl/hbproces.c index 106119cde2..fed8e5ab53 100644 --- a/harbour/src/rtl/hbproces.c +++ b/harbour/src/rtl/hbproces.c @@ -318,7 +318,7 @@ static int hb_fsProcessExec( const char * pszFilename, if( iMaxFD < 3 ) iMaxFD = 1024; for( i = 3; i < iMaxFD; ++i ) - close( i ); + hb_fsClose( i ); } /* reset extended process attributes */ setuid( getuid() ); @@ -422,18 +422,30 @@ HB_FHANDLE hb_fsProcessOpen( const char * pszFilename, sa.bInheritHandle = HB_TRUE; if( phStdin != NULL ) - fError = !CreatePipe( &hPipes[0], &hPipes[1], &sa, 0 ); + { + fError = !CreatePipe( &hPipes[ 0 ], &hPipes[ 1 ], &sa, 0 ); + if( !fError ) + SetHandleInformation( hPipes[ 1 ], HANDLE_FLAG_INHERIT, 0 ); + } if( !fError && phStdout != NULL ) - fError = !CreatePipe( &hPipes[2], &hPipes[3], &sa, 0 ); + { + fError = !CreatePipe( &hPipes[ 2 ], &hPipes[ 3 ], &sa, 0 ); + if( !fError ) + SetHandleInformation( hPipes[ 2 ], HANDLE_FLAG_INHERIT, 0 ); + } if( !fError && phStderr != NULL ) { if( phStdout == phStderr ) { - hPipes[4] = hPipes[2]; - hPipes[5] = hPipes[3]; + hPipes[ 4 ] = hPipes[ 2 ]; + hPipes[ 5 ] = hPipes[ 3 ]; } else - fError = CreatePipe( &hPipes[4], &hPipes[5], &sa, 0 ) ? HB_FALSE : HB_TRUE; + { + fError = !CreatePipe( &hPipes[ 4 ], &hPipes[ 5 ], &sa, 0 ); + if( !fError ) + SetHandleInformation( hPipes[ 4 ], HANDLE_FLAG_INHERIT, 0 ); + } } if( fError ) @@ -515,7 +527,8 @@ HB_FHANDLE hb_fsProcessOpen( const char * pszFilename, CloseHandle( hPipes[ i ] ); } } -#elif defined( HB_OS_UNIX ) && !defined( HB_OS_VXWORKS ) && !defined( HB_OS_SYMBIAN ) +#elif defined( HB_OS_UNIX ) && \ + !defined( HB_OS_VXWORKS ) && !defined( HB_OS_SYMBIAN ) { HB_BOOL fError = HB_FALSE; HB_FHANDLE hPipeIn [ 2 ] = { FS_ERROR, FS_ERROR }, @@ -578,15 +591,25 @@ HB_FHANDLE hb_fsProcessOpen( const char * pszFilename, dup2( hNull, 2 ); if( hNull != FS_ERROR ) - close( hNull ); + hb_fsClose( hNull ); } if( phStdin != NULL ) + { dup2( hPipeIn[ 0 ], 0 ); + hb_fsClose( hPipeIn[ 1 ] ); + } if( phStdout != NULL ) + { dup2( hPipeOut[ 1 ], 1 ); + hb_fsClose( hPipeOut[ 0 ] ); + } if( phStderr != NULL ) + { dup2( hPipeErr[ 1 ], 2 ); + if( phStdout != phStderr ) + hb_fsClose( hPipeErr[ 0 ] ); + } /* close all non std* handles */ { @@ -595,7 +618,7 @@ HB_FHANDLE hb_fsProcessOpen( const char * pszFilename, if( iMaxFD < 3 ) iMaxFD = 1024; for( i = 3; i < iMaxFD; ++i ) - close( i ); + hb_fsClose( i ); } /* reset extended process attributes */ @@ -631,19 +654,19 @@ HB_FHANDLE hb_fsProcessOpen( const char * pszFilename, hb_fsSetIOError( !fError, 0 ); if( hPipeIn[ 0 ] != FS_ERROR ) - close( hPipeIn[ 0 ] ); + hb_fsClose( hPipeIn[ 0 ] ); if( hPipeIn[ 1 ] != FS_ERROR ) - close( hPipeIn[ 1 ] ); + hb_fsClose( hPipeIn[ 1 ] ); if( hPipeOut[ 0 ] != FS_ERROR ) - close( hPipeOut[ 0 ] ); + hb_fsClose( hPipeOut[ 0 ] ); if( hPipeOut[ 1 ] != FS_ERROR ) - close( hPipeOut[ 1 ] ); + hb_fsClose( hPipeOut[ 1 ] ); if( phStdout != phStderr ) { if( hPipeErr[ 0 ] != FS_ERROR ) - close( hPipeErr[ 0 ] ); + hb_fsClose( hPipeErr[ 0 ] ); if( hPipeErr[ 1 ] != FS_ERROR ) - close( hPipeErr[ 1 ] ); + hb_fsClose( hPipeErr[ 1 ] ); } } #elif defined( HB_OS_OS2 ) || defined( HB_OS_WIN ) @@ -916,6 +939,8 @@ HB_BOOL hb_fsProcessClose( HB_FHANDLE hProcess, HB_BOOL fGentle ) if( TerminateProcess( hProc, fGentle ? 0 : 1 ) ) fResult = HB_TRUE; hb_fsSetIOError( fResult, 0 ); + /* hProc has to be closed by hb_fsProcessValue() */ + /* CloseHandle( hProc ); */ } else hb_fsSetError( ( HB_ERRCODE ) FS_ERROR ); @@ -1156,6 +1181,8 @@ int hb_fsProcessRun( const char * pszFilename, if( hStderr != FS_ERROR ) hb_fsClose( hStderr ); + CloseHandle( ( HANDLE ) hb_fsGetOsHandle( hProcess ) ); + #elif defined( HB_OS_UNIX ) && !defined( HB_OS_SYMBIAN ) fd_set rfds, wfds, *prfds, *pwfds;