Re: (ITS#8504) LDMB: Return EPIPE from mdb_env_copyfd2 instead abort on SIGPIPE

[lmb]

--001a114741aec1bad3053f481b06
Content-Type: text/plain; charset=UTF-8

On 20 October 2016 at 09:35, Hallvard Breien Furuseth <
h.b.furus...@usit.uio.no> wrote:
>
> Hmm.  Nevermind, it's probably better to leave that to the user.
> It gets ugly for a library to meddle with the current thread's
> signals.  I.e. it must check if SIGPIPE was already pending
> before the call, and don't collect it on EPIPE in that case.
>

Yeah, I agree. Your changes LGTM.

Interesting point re atomic int write, I wasn't entirely sure how mc_error
could be accesses, so I decided to play it safe.

-- 
Lorenz Bauer  |  Systems Engineer
25 Lavington St., London SE1 0NZ

www.cloudflare.com

--001a114741aec1bad3053f481b06
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

=
On 20 October 2016 at 09:35, Hallvard Breien Furuseth =
;mailto:h.b.furus...@usit.uio.no; target=3D"_blank">h.b.furuseth=
@usit.uio.no wrote:
Hmm.=C2=A0 Nevermind, its probably better to leave that to the 

Re: (ITS#8504) LDMB: Return EPIPE from mdb_env_copyfd2 instead abort on SIGPIPE

[h . b . furuseth]

I wrote:
> And.. duh, we've forgotten mdb_copy without MDB_CP_COMPACT.
> I'll do it later if nobody beats me to it.

Hmm.  Nevermind, it's probably better to leave that to the user.
It gets ugly for a library to meddle with the current thread's
signals.  I.e. it must check if SIGPIPE was already pending
before the call, and don't collect it on EPIPE in that case.

-- 
Hallvard

Re: (ITS#8504) LDMB: Return EPIPE from mdb_env_copyfd2 instead abort on SIGPIPE

[h . b . furuseth]

On 27/09/16 22:14, l...@cloudflare.com wrote:
>On 26 September 2016 at 20:48, Hallvard Breien Furuseth 
> wrote:
>> I think we can skip sigwait() too since the thread will exit when done.
>> (...)
>
> That's what my initial patch did. At least on OS X this leads to the
> process receiving SIGPIPE and dying.

OK.  I've put a fixed version including that comment in branch
"mdb/its8504-sigpipe" @ .
We can squash the commits later:

Never clear mc_error, we'd lose failure in the other thread.
It's not mutex-protected, which is OK: If both threads set it,
we'll get one of the errors. LMDB already expects atomic int.

And.. duh, we've forgotten mdb_copy without MDB_CP_COMPACT.
I'll do it later if nobody beats me to it.
Test:  Comment out SIGPIPE in mdb_copy.c, then run

   bash$ (sleep 1; ./mdb_copy testdb; echo exit $? >&2) | true
   exit 141

Exit values >= 128 are aborts from signals.

(ITS#8504) LDMB: Return EPIPE from mdb_env_copyfd2 instead abort on SIGPIPE

[lmb]

On 26 September 2016 at 20:48, Hallvard Breien Furuseth
 wrote:
> I think we can skip sigwait() too since the thread will exit when done.
> Then any pending signals are presumably dropped.  (I suppose some OS
> could give them to another thread, but that goes against delivering
> them to a particular thread like SIGPIPE is.)

That's what my initial patch did. At least on OS X this leads to the
process receiving SIGPIPE and dying. I can attach a test case as well
if desired.

Re: (ITS#8504) LDMB: Return EPIPE from mdb_env_copyfd2 instead abort on SIGPIPE

[h . b . furuseth]

On 25/09/16 20:16, l...@cloudflare.com wrote:
> * Do not check for pending SIGPIPE: since this is a new thread there can't
> be any

I think we can skip sigwait() too since the thread will exit when done.
Then any pending signals are presumably dropped.  (I suppose some OS
could give them to another thread, but that goes against delivering
them to a particular thread like SIGPIPE is.)

Re: (ITS#8504) LDMB: Return EPIPE from mdb_env_copyfd2 instead abort on SIGPIPE

[lmb]

--94eb2c07d07472c185053d5903fe
Content-Type: text/plain; charset=UTF-8

I've updated the patch to be much simpler:

* Set pthread_sigmask in copythr and return an error in my->mc_error
* Do not check for pending SIGPIPE: since this is a new thread there can't
be any

The URL stays the same.

--94eb2c07d07472c185053d5903fe
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Ive updated the patch to b=
e much simpler:* Set pthread_sigmask in copythr and return an error in my-mc=
_error* Do not check for pending SIGPIPE: =
since this is a new thread there cant be anyThe URL stays the same.

--94eb2c07d07472c185053d5903fe--

(ITS#8504) LDMB: Return EPIPE from mdb_env_copyfd2 instead abort on SIGPIPE

[lmb]

Full_Name: Lorenz Bauer
Version: 
OS: 
URL: https://gist.github.com/lmb/63189f83c5f00dd59c86f3c9bc07694d
Submission from: (NULL) (2606:4700:1000:8200:5530:fc35:2f95:4f31)


As per the discussion in [1] I'm providing a patch to block and handle SIGPIPE
in the copy thread used when performing a compacting copy.

The attached file is derived from OpenLDAP Software. All of the modifications to
OpenLDAP Software represented in the following patch(es) were developed by
CloudFlare, Inc. CloudFlare, Inc. has not assigned rights and/or interest in
this work to any party. I, Lorenz Bauer am authorized by CloudFlare, Inc., my
employer, to release this work under the following terms.
CloudFlare, Inc. hereby place the following modifications to OpenLDAP Software
(and only these modifications) into the public domain. Hence, these
modifications may be freely used and/or redistributed for any purpose with or
without attribution and/or other notice.

1: http://www.openldap.org/lists/openldap-technical/201609/msg00026.html