Re: Win32 shmem - Mailing list pgsql-patches

From Tom Lane
Subject Re: Win32 shmem
Date
Msg-id 2510.1174355191@sss.pgh.pa.us
Whole thread Raw
In response to Win32 shmem  (Magnus Hagander <magnus@hagander.net>)
List pgsql-patches
Magnus Hagander <magnus@hagander.net> writes:
> Attached is a first attempt at a "native win32 shmem implementatio",
> based on previous discussions. Instead of emulating the sysv stuff. The
> base stuff (using mapped pagefile) is still the same, of course.

> Needs more testing, but has survived my tests so far. And unlike the
> previous implementation, it does properly refuse to start a second
> postmaster in a data directory where there is one or more backends still
> running.

That's good...

> Does it seem like I've overlooked anything obvious in this? I do get the
> feeling that this is too simple, but I don't know exactly where the
> problem is :-)

I think you do still need the on_shmem_exit detach callback.  Consider
the situation where the postmaster is trying to reinitialize after a
child crash.  The Unix code is designed to detach and destroy the old
segment then create a new one.  If that's not what you want to do then
this code still seems not right.

The coding of GetShareMemName seems involuted.  I'd normally expect
success exit to be at the bottom of the routine, not deeply nested
like this.  You may be saving one elog(FATAL) call this way, but
I'd argue that the two cases could do with different message texts
anyway.  (Also, GetSharedMemName seems to read better.)

There seem to be a lot of system calls not checked for failure here.
Do they really not have any failure possibilities?

>                 errdetail("Failed system call was MapViewOfFileEx", size, szShareMem)));

Doesn't your compiler validate sprintf arguments?

I trust the committed file isn't going to have DOS line endings.

            regards, tom lane

pgsql-patches by date:

Previous
From: Tom Lane
Date:
Subject: Re: bgwriter stats
Next
From: Darcy Buskermolen
Date:
Subject: Re: bgwriter stats