Re: msvcrt: Don't hold FILE critical section in some functions

[Alexandre Julliard]

Piotr Caban <piotr(a)codeweavers.com> writes:
> @@ -3117,25 +3098,20 @@ int CDECL MSVCRT_fputc(int c, MSVCRT_FILE* file)
>   */
>  int CDECL MSVCRT__flsbuf(int c, MSVCRT_FILE* file)
>  {
> -    MSVCRT__lock_file(file);
> -
>      /* Flush output buffer */
>      if(file->_bufsiz == 0 && !(file->_flag & MSVCRT__IONBF)) {
>          msvcrt_alloc_buffer(file);
>      }
>      if(!(file->_flag & MSVCRT__IOWRT)) {
> -        if(file->_flag & MSVCRT__IORW) {
> +        if(file->_flag & MSVCRT__IORW)
>              file->_flag |= MSVCRT__IOWRT;
> -        } else {
> -            MSVCRT__unlock_file(file);
> +        else

That doesn't look safe.

-- 
Alexandre Julliard
julliard(a)winehq.org

msvcrt: Don't hold FILE critical section in some functions

[Piotr Caban]

On 11/15/11 16:34, Alexandre Julliard wrote:
> Piotr Caban<piotr(a)codeweavers.com>  writes:
> > @@ -3117,25 +3098,20 @@ int CDECL MSVCRT_fputc(int c, MSVCRT_FILE* file)
> >    */
> >   int CDECL MSVCRT__flsbuf(int c, MSVCRT_FILE* file)
> >   {
> > -    MSVCRT__lock_file(file);
> > -
> >       /* Flush output buffer */
> >       if(file->_bufsiz == 0&&  !(file->_flag&  MSVCRT__IONBF)) {
> >           msvcrt_alloc_buffer(file);
> >       }
> >       if(!(file->_flag&  MSVCRT__IOWRT)) {
> > -        if(file->_flag&  MSVCRT__IORW) {
> > +        if(file->_flag&  MSVCRT__IORW)
> >               file->_flag |= MSVCRT__IOWRT;
> > -        } else {
> > -            MSVCRT__unlock_file(file);
> > +        else
>
> That doesn't look safe.

This function is not safe under Windows 7. I was testing it with 
following program:

static DWORD WINAPI block_stdin(void *arg)
{
	_lock_file(stdout);
	return 0;
}

int main( )
{
	DWORD thread_id;

	CreateThread(NULL, 0, block_stdin, NULL, 0, &thread_id);
	Sleep(500);
	_flsbuf(32, stdout);
	return 0;
}

It terminates in Windows 7. It never ends with Wine both with and 
without my patch (because of locks in fputc and fwrite).

msvcrt: Don't hold FILE critical section in some functions

[Alexandre Julliard]

Piotr Caban <piotr(a)codeweavers.com> writes:
> This function is not safe under Windows 7. I was testing it with
> following program:
>
> static DWORD WINAPI block_stdin(void *arg)
> {
>   _lock_file(stdout);
>   return 0;
> }
>
> int main( )
> {
>   DWORD thread_id;
>
> CreateThread(NULL, 0, block_stdin, NULL, 0, &thread_id);
>   Sleep(500);
>   _flsbuf(32, stdout);
>   return 0;
> }
>
> It terminates in Windows 7. It never ends with Wine both with and
> without my patch (because of locks in fputc and fwrite).

OK then. Are you planning to add test cases for this?

-- 
Alexandre Julliard
julliard(a)winehq.org