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.

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--