Обсуждение: pgsql: Fix a couple of bugs in win32 shmem name generation: * Don't cut

Log Message:
-----------
Fix a couple of bugs in win32 shmem name generation:
* Don't cut off the prefix. With this fix, it's again readable.
* Properly store it in the Global namespace as intended.

Tags:
----
REL8_3_STABLE

Modified Files:
--------------
    pgsql/src/backend/port:
        win32_shmem.c (r1.4 -> r1.4.2.1)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/port/win32_shmem.c?r1=1.4&r2=1.4.2.1)

Magnus Hagander wrote:
> Log Message:
> -----------
> Fix a couple of bugs in win32 shmem name generation:
> * Don't cut off the prefix. With this fix, it's again readable.
> * Properly store it in the Global namespace as intended.
>
> Tags:
> ----
> REL8_3_STABLE

What happens if someone launches version 8.3.3 postgres.exe and version
8.3.4 postgres.exe at the same time, on the same data directory? Will
the interlock that prevents two postmasters from starting at the same
time work?

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

Heikki Linnakangas wrote:
> Magnus Hagander wrote:
>> Log Message:
>> -----------
>> Fix a couple of bugs in win32 shmem name generation:
>> * Don't cut off the prefix. With this fix, it's again readable.
>> * Properly store it in the Global namespace as intended.
>>
>> Tags:
>> ----
>> REL8_3_STABLE
>
> What happens if someone launches version 8.3.3 postgres.exe and version
> 8.3.4 postgres.exe at the same time, on the same data directory? Will
> the interlock that prevents two postmasters from starting at the same
> time work?

Hmm. Didn't think of that :(

Yeah, it seems that that part of it would fail. In a lot of cases you'd
still get kicked off by socket conflicts and such, but the shared memory
part would not notice someone is already there, no.

Not sure if it's reason enough to revert - since it fixes other cases. I
guess in theory we could check both the old and the new name, but that's
going to be a considerably more complex patch.

//Magnus

Magnus Hagander wrote:
> Heikki Linnakangas wrote:
>> Magnus Hagander wrote:
>>> Fix a couple of bugs in win32 shmem name generation:
>>> * Don't cut off the prefix. With this fix, it's again readable.
>>> * Properly store it in the Global namespace as intended.
>>>
>>> Tags:
>>> ----
>>> REL8_3_STABLE
>> What happens if someone launches version 8.3.3 postgres.exe and version
>> 8.3.4 postgres.exe at the same time, on the same data directory? Will
>> the interlock that prevents two postmasters from starting at the same
>> time work?
>
> Hmm. Didn't think of that :(
>
> Yeah, it seems that that part of it would fail. In a lot of cases you'd
> still get kicked off by socket conflicts and such, but the shared memory
> part would not notice someone is already there, no.

Yeah.

> Not sure if it's reason enough to revert - since it fixes other cases. I
> guess in theory we could check both the old and the new name, but that's
> going to be a considerably more complex patch.

Yeah, and it would still fail if you started the new version first, and
then the old one.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

Magnus Hagander <magnus@hagander.net> writes:
> Heikki Linnakangas wrote:
>> What happens if someone launches version 8.3.3 postgres.exe and version
>> 8.3.4 postgres.exe at the same time, on the same data directory? Will
>> the interlock that prevents two postmasters from starting at the same
>> time work?

> Hmm. Didn't think of that :(

> Yeah, it seems that that part of it would fail. In a lot of cases you'd
> still get kicked off by socket conflicts and such, but the shared memory
> part would not notice someone is already there, no.

> Not sure if it's reason enough to revert - since it fixes other cases. I
> guess in theory we could check both the old and the new name, but that's
> going to be a considerably more complex patch.

According to what you just told me, the original coding is storing the
name in a "local namespace", which presumably means it won't conflict
anyway.  Ergo, the existing coding is simply broken and there's nothing
we can do about it.

            regards, tom lane

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Heikki Linnakangas wrote:
>>> What happens if someone launches version 8.3.3 postgres.exe and version
>>> 8.3.4 postgres.exe at the same time, on the same data directory? Will
>>> the interlock that prevents two postmasters from starting at the same
>>> time work?
>
>> Hmm. Didn't think of that :(
>
>> Yeah, it seems that that part of it would fail. In a lot of cases you'd
>> still get kicked off by socket conflicts and such, but the shared memory
>> part would not notice someone is already there, no.
>
>> Not sure if it's reason enough to revert - since it fixes other cases. I
>> guess in theory we could check both the old and the new name, but that's
>> going to be a considerably more complex patch.
>
> According to what you just told me, the original coding is storing the
> name in a "local namespace", which presumably means it won't conflict
> anyway.  Ergo, the existing coding is simply broken and there's nothing
> we can do about it.

Local namespace = Session local, not process local. So it would properly
protect against two processes started in the same session. One session
is, for example, an interactive login. But not if they were started by
different users, since they'd be in different sessions.

//Magnus

Magnus Hagander wrote:
> Tom Lane wrote:

> > According to what you just told me, the original coding is storing the
> > name in a "local namespace", which presumably means it won't conflict
> > anyway.  Ergo, the existing coding is simply broken and there's nothing
> > we can do about it.
>
> Local namespace = Session local, not process local. So it would properly
> protect against two processes started in the same session. One session
> is, for example, an interactive login. But not if they were started by
> different users, since they'd be in different sessions.

But those different users would not have access to the same set of
files, so it wouldn't work anyway, right?

--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera wrote:
> Magnus Hagander wrote:
>> Tom Lane wrote:
>
>>> According to what you just told me, the original coding is storing the
>>> name in a "local namespace", which presumably means it won't conflict
>>> anyway.  Ergo, the existing coding is simply broken and there's nothing
>>> we can do about it.
>> Local namespace = Session local, not process local. So it would properly
>> protect against two processes started in the same session. One session
>> is, for example, an interactive login. But not if they were started by
>> different users, since they'd be in different sessions.
>
> But those different users would not have access to the same set of
> files, so it wouldn't work anyway, right?

Depends on what permissions you set on the directory, obviously....
Default ones depend on windows version and where in the filesystem they go.

//Magnus