Re: Alexandre Julliard : msvcrt: Don't bother to clean up at process exit.

[Dmitry Timoshkov]

Alexandre Julliard <julliard(a)winehq.org> wrote:
> case DLL_PROCESS_DETACH:
> -    msvcrt_free_popen_data();
>      msvcrt_free_io();
> +    if (lpvReserved) break;
> +    msvcrt_free_popen_data();
>      msvcrt_free_mt_locks();
>      msvcrt_free_console();
>      msvcrt_free_args();

Shouldn't msvcrt_free_io() be also protected by this guard?

-- 
Dmitry.

Alexandre Julliard : msvcrt: Don't bother to clean up at process exit.

[Alexandre Julliard]

Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
> Alexandre Julliard <julliard(a)winehq.org> wrote:
> > case DLL_PROCESS_DETACH:
> > -    msvcrt_free_popen_data();
> >      msvcrt_free_io();
> > +    if (lpvReserved) break;
> > +    msvcrt_free_popen_data();
> >      msvcrt_free_mt_locks();
> >      msvcrt_free_console();
> >      msvcrt_free_args();
>
> Shouldn't msvcrt_free_io() be also protected by this guard?

I think we still want to try to flush the file buffers.

-- 
Alexandre Julliard
julliard(a)winehq.org

Alexandre Julliard : msvcrt: Don't bother to clean up at process exit.

[Dmitry Timoshkov]

Alexandre Julliard <julliard(a)winehq.org> wrote:
> > > case DLL_PROCESS_DETACH:
> > > -    msvcrt_free_popen_data();
> > >      msvcrt_free_io();
> > > +    if (lpvReserved) break;
> > > +    msvcrt_free_popen_data();
> > >      msvcrt_free_mt_locks();
> > >      msvcrt_free_console();
> > >      msvcrt_free_args();
> >
> > Shouldn't msvcrt_free_io() be also protected by this guard?
>
> I think we still want to try to flush the file buffers.

My point was that msvcrt_free_io() does much more than just flushing
file buffers, it also frees memory and deletes critical sections.

-- 
Dmitry.

Alexandre Julliard : msvcrt: Don't bother to clean up at process exit.

[Alexandre Julliard]

Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
> Alexandre Julliard <julliard(a)winehq.org> wrote:
> > > > case DLL_PROCESS_DETACH:
> > > > -    msvcrt_free_popen_data();
> > > >      msvcrt_free_io();
> > > > +    if (lpvReserved) break;
> > > > +    msvcrt_free_popen_data();
> > > >      msvcrt_free_mt_locks();
> > > >      msvcrt_free_console();
> > > >      msvcrt_free_args();
> > >
> > > Shouldn't msvcrt_free_io() be also protected by this guard?
> >
> > I think we still want to try to flush the file buffers.
>
> My point was that msvcrt_free_io() does much more than just flushing
> file buffers, it also frees memory and deletes critical sections.

It should probably call fflush or flushall instead, but that doesn't
flush stdio. I'm not quite convinced that this is correct, it would need
test cases.

-- 
Alexandre Julliard
julliard(a)winehq.org

Alexandre Julliard : msvcrt: Don't bother to clean up at process exit.

[Christian Costa]

Out of curiosity. Not freeing these memory allocations shouldn't disturb
Valgrind to report false memory leaks?


2013/5/21 Alexandre Julliard <julliard(a)winehq.org>
> Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
> > Alexandre Julliard <julliard(a)winehq.org> wrote:
> > > > > case DLL_PROCESS_DETACH:
> > > > > -    msvcrt_free_popen_data();
> > > > >      msvcrt_free_io();
> > > > > +    if (lpvReserved) break;
> > > > > +    msvcrt_free_popen_data();
> > > > >      msvcrt_free_mt_locks();
> > > > >      msvcrt_free_console();
> > > > >      msvcrt_free_args();
> > > >
> > > > Shouldn't msvcrt_free_io() be also protected by this guard?
> > >
> > > I think we still want to try to flush the file buffers.
> >
> > My point was that msvcrt_free_io() does much more than just flushing
> > file buffers, it also frees memory and deletes critical sections.
>
> It should probably call fflush or flushall instead, but that doesn't
> flush stdio. I'm not quite convinced that this is correct, it would need
> test cases.
>
> --
> Alexandre Julliard
> julliard(a)winehq.org

Alexandre Julliard : msvcrt: Don't bother to clean up at process exit.

[Dmitry Timoshkov]

Christian Costa <titan.costa(a)gmail.com> wrote:
> Out of curiosity. Not freeing these memory allocations shouldn't disturb
> Valgrind to report false memory leaks?

Avoiding heap access on process exit should avoid possible dead locks
in case a terminated by ExitProcess thread holds the heap lock.

-- 
Dmitry.

Alexandre Julliard : msvcrt: Don't bother to clean up at process exit.

[Christian Costa]

That's ok with me.Thanks for the explanation. So Valgrind will report these
"intentional" memory leaks in addition to the "unintentional" ones. Correct?


2013/5/21 Dmitry Timoshkov <dmitry(a)baikal.ru>
> Christian Costa <titan.costa(a)gmail.com> wrote:
> > Out of curiosity. Not freeing these memory allocations shouldn't disturb
> > Valgrind to report false memory leaks?
>
> Avoiding heap access on process exit should avoid possible dead locks
> in case a terminated by ExitProcess thread holds the heap lock.
>
> --
> Dmitry.