Re: [HACKERS] Recursive ReceiveSharedInvalidMessages not safe

2014-05-08 Thread Andres Freund
On 2014-05-05 15:41:22 -0400, Tom Lane wrote: After far, far too much confused head scratching, code reading, random elog()s et al I found out that this is just because of a deficiency in valgrind's undefinedness tracking. [...] Unfortunately this cannot precisely be caught by valgrind's

Re: [HACKERS] Recursive ReceiveSharedInvalidMessages not safe

2014-05-08 Thread Andres Freund
On 2014-05-05 18:50:59 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-05-05 15:41:22 -0400, Tom Lane wrote: Looks all right to me. Yeah, the right shift might have undefined high-order bits, but we don't care because we're storing the result into an int16.

[HACKERS] Recursive ReceiveSharedInvalidMessages not safe

2014-05-05 Thread Andres Freund
Hi, While investigating an issue pointed out by valgrind around undefined bytes in inval.c SHAREDINVALSMGR_ID processing I noticed that there's a bug in ReceiveSharedInvalidMessages(). It tries to be safe against recursion but it's not: When it recurses into ReceiveSharedInvalidMessages() from

Re: [HACKERS] Recursive ReceiveSharedInvalidMessages not safe

2014-05-05 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: While investigating an issue pointed out by valgrind around undefined bytes in inval.c SHAREDINVALSMGR_ID processing I noticed that there's a bug in ReceiveSharedInvalidMessages(). It tries to be safe against recursion but it's not: When it

Re: [HACKERS] Recursive ReceiveSharedInvalidMessages not safe

2014-05-05 Thread Andres Freund
On 2014-05-05 14:15:58 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: While investigating an issue pointed out by valgrind around undefined bytes in inval.c SHAREDINVALSMGR_ID processing I noticed that there's a bug in ReceiveSharedInvalidMessages(). It tries to be safe

Re: [HACKERS] Recursive ReceiveSharedInvalidMessages not safe

2014-05-05 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: a) SICleanupQueue() sometimes releases and reacquires the write lock held on the outside. That's pretty damn fragile, not to mention ugly. Even slight reformulations of the code in SIInsertDataEntries() can break this... Can we please

Re: [HACKERS] Recursive ReceiveSharedInvalidMessages not safe

2014-05-05 Thread Andres Freund
On 2014-05-05 15:41:22 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: a) SICleanupQueue() sometimes releases and reacquires the write lock held on the outside. That's pretty damn fragile, not to mention ugly. Even slight reformulations of the code in

Re: [HACKERS] Recursive ReceiveSharedInvalidMessages not safe

2014-05-05 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: On 2014-05-05 15:41:22 -0400, Tom Lane wrote: Looks all right to me. Yeah, the right shift might have undefined high-order bits, but we don't care because we're storing the result into an int16. Doesn't at the very least