From: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-committers(at)postgresql(dot)org |
Subject: | Re: pgsql: Fix race condition in pg_ctl reading postmaster.pid. |
Date: | 2012-10-15 14:33:00 |
Message-ID: | 507C1E9C.8050106@vmware.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
On 15.10.2012 17:05, Tom Lane wrote:
> Heikki Linnakangas<heikki(dot)linnakangas(at)iki(dot)fi> writes:
>> Fix race condition in pg_ctl reading postmaster.pid.
>> If postmaster changed postmaster.pid while pg_ctl was reading it, pg_ctl
>> could overrun the buffer it allocated for the file. Fix by reading the
>> whole file to memory with one read() call.
>
> Maybe I'm just not awake enough, but that code doesn't look to me like
> it does the right thing with a non-newline-terminated file. Doesn't it
> drop the last character of the last line?
No. It loops until the next-to-last character when it searches for
newlines. When it hits that, it creates the final string to return,
which includes the final character, whether it's a newline or not.
BTW, an easy way to test this is to edit postmaster.opts on a running
server, and do "pg_ctl status". It prints out the contents of
postmaster.opts, as parsed by the function.
> Given the way that pg_ctl uses the file, I think that the old logic of
> "pretend the file ends with a newline" is wrong anyway. If we do manage
> to see an intermediate state of the file, it would be better to not
> return the last line at all than to return a truncated (a/k/a wrong)
> value for that line. So I'd vote to rejigger the logic to ignore any
> data after the last newline.
Good point. I'll rejigger..
- Heikki
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2012-10-15 16:19:25 | pgsql: alter_generic regression test cannot run concurrently with privi |
Previous Message | Tom Lane | 2012-10-15 14:05:39 | Re: pgsql: Fix race condition in pg_ctl reading postmaster.pid. |