From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Unexpected "shared memory block is still in use"
Date: Fri, 10 May 2019 00:22:13 -0700
Message-ID: <20190510072213.GA1098093@rfd.leadboat.com> (raw)
In-Reply-To: <17642.1557442078@sss.pgh.pa.us>
References: <22224.1557340366@sss.pgh.pa.us>
<20190509055414.GB1066859@rfd.leadboat.com>
<2663.1557409654@sss.pgh.pa.us>
<2226.1557439179@sss.pgh.pa.us>
<17642.1557442078@sss.pgh.pa.us>
On Thu, May 09, 2019 at 06:47:58PM -0400, Tom Lane wrote:
> I wrote:
> > However, I have a new theory, after noticing that c09850992 moved the
> > check for shm_nattch == 0. Previously, if a shmem segment had zero attach
> > count, it was unconditionally considered not-a-threat. Now, we'll try
> > shmat() anyway, and if that fails for any reason other than EACCES, we say
> > SHMSTATE_ANALYSIS_FAILURE which leads to the described error report.
> > So I suspect that what we hit was a race condition whereby some other
> > parallel test was using the same shmem ID and we managed to see its
> > segment successfully in shmctl but then it was gone by the time we did
> > shmat. This leads me to think that EINVAL and EIDRM failures from
> > shmat had better be considered SHMSTATE_ENOENT not
> > SHMSTATE_ANALYSIS_FAILURE.
> > In principle this is a longstanding race condition, but I wonder
> > whether we made it more probable by moving the shm_nattch check.
>
> Hah --- this is a real race condition, and I can demonstrate it very
> easily by inserting a sleep right there, as in the attached
> for-testing-only patch.
>
> The particular parallelism level I use is
>
> make -s check-world -j4 PROVE_FLAGS='-j4 --quiet --nocolor --nocount'
>
> on a dual-socket 4-cores-per-socket Xeon machine. With that command and
> this patch, I frequently get multiple failures per run, and they all
> report either EINVAL or EIDRM.
>
> The patch generally reports that nattch had been 1, so my thought that
> that change might've made it worse seems unfounded. But we have
> absolutely got a hittable race condition here. The real fix should
> be on the order of
>
> if (errno == EACCES)
> return SHMSTATE_FOREIGN;
> + else if (errno == EINVAL || errno == EIDRM)
> + return SHMSTATE_ENOENT;
> else
> return SHMSTATE_ANALYSIS_FAILURE;
>
> (plus comments of course).
Looks good. That is basically a defect in commit c09850992; the race passed
from irrelevance to relevance when that commit subjected more segments to the
test. Thanks for diagnosing it.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: noah@leadboat.com, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: Unexpected "shared memory block is still in use"
In-Reply-To: <20190510072213.GA1098093@rfd.leadboat.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox