Re: Odd behavior with PG_TRY

On 1/2/17 9:47 PM, Tom Lane wrote:
> Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com> writes:
>> In the attached patch (snippet below), I'm seeing something strange with
>> args->in.r.atts[].
>
> Did you try comparing the apparent values of "args" before and after
> entering PG_TRY?

Yeah, see below. FWIW, when I did that just now I stepped through at the
instruction level, and things went belly up when I stepped into the
first instruction of the for (first instruction after PG_TRY was done).

>> I saw the comment on PG_TRY about marking things as volatile, but my
>> understanding from the comment is I shouldn't even need to do that,
>> since these variables aren't being modified.
>
> Not sure, but if you do need to mark them volatile to prevent
> misoptimization in the PG_TRY, this is not how to do it:
>
>> volatile TupleDesc desc = slot->tts_tupleDescriptor;
>> volatile CallbackState *myState = (CallbackState *) self;
>> volatile PLyTypeInfo *args = myState->args;
>
> Correct coding would be
>
> volatile TupleDesc desc = slot->tts_tupleDescriptor;
> CallbackState * volatile myState = (CallbackState *) self;
> PLyTypeInfo * volatile args = myState->args;
>
> because what needs to be marked volatile is the pointer variable,
> not what it points at. I'm a bit surprised you're not getting
> "cast away volatile" warnings from the code as you have it.

Unfortunately, that didn't make a difference. Amit's suggestion of
isolating the single statement in a PG_TRY() didn't work either, but
assigning args->in.r.atts[i] to a pointer did. The volatile didn't make
a difference in this case, which if the PG_TRY() comments are to be
believed is what I'd expect. Though, it was also OK with value not being
volatile, which AFAIK isn't safe, so...

for (i = 0; i < desc->natts; i++)
{
PyObject *value = NULL;
PLyDatumToOb * volatile atts = &args->in.r.atts[i];

...

if (slot->tts_isnull[i] || atts->func == NULL)
{
Py_INCREF(Py_None);
value = Py_None;
}
else
{
PG_TRY();
{
value = (atts->func) (atts, slot->tts_values[i]);
}
PG_CATCH();
{
Py_XDECREF(value);
MemoryContextSwitchTo(oldcontext);
PLy_switch_execution_context(old_exec_ctx);
PG_RE_THROW();
}
PG_END_TRY();
}

lldb output from inspecting variables:

(lldb) call args
(volatile PLyTypeInfo *) $60 = 0x00007fff5912fbe8
(lldb) call @args
error: expected expression
(lldb) call *args
(PLyTypeInfo) $61 = {
in = {
d = {
func = 0x00007fca218f8ed0
typfunc = {
fn_addr = 0x00007fff00000001
fn_oid = 114610686
fn_nargs = 1
fn_strict = '\0'
fn_retset = '\0'
fn_stats = '0'
fn_extra = 0x00007fca2206d060
fn_mcxt = 0x00007fca22800960
fn_expr = 0x00007fff5912fbe8
}
typtransform = {
fn_addr = 0x00007fca218f8e38
fn_oid = 570849696
fn_nargs = 32714
fn_strict = '\0'
fn_retset = '\0'
fn_stats = '8'
fn_extra = 0x00007fca22066f60
fn_mcxt = 0x00007fff5912fc90
fn_expr = 0x0000000106d7df6f
}
typoid = 570830824
typmod = 32714
typioparam = 570830864
typbyval = '\0'
typlen = 0
typalign = 'h'
elm = 0x0000000000000000
}
r = {
atts = 0x00007fca218f8ed0
natts = 1
}
}
out = {
d = {
func = 0x00007fca22066f60
typfunc = {
fn_addr = 0x00007fca00000000
fn_oid = 570846544
fn_nargs = 32714
fn_strict = '\0'
fn_retset = '\0'
fn_stats = '`'
fn_extra = 0x00007fff5912fce0
fn_mcxt = 0x0000000106d4d437
fn_expr = 0x00007fca2206ce30
}
typtransform = {
fn_addr = 0x00007fca22062f90
fn_oid = 0
fn_nargs = 0
fn_strict = '\0'
fn_retset = '\0'
fn_stats = '\0'
fn_extra = 0xffffffdc22066c20
fn_mcxt = 0x0000000000000000
fn_expr = 0x00007fca22066f60
}
typoid = 3756919206
typmod = -754934605
typioparam = 1494416704
typbyval = '\xff'
typlen = 0
typalign = '^'
elm = 0x00007fff5912fd10
}
r = {
atts = 0x00007fca22066f60
natts = 0
}
}
is_rowtype = 0
typ_relid = 0
typrel_xmin = 570847072
typrel_tid = {
ip_blkid = (bi_hi = 32714, bi_lo = 0)
ip_posid = 36408
}
mcxt = 0x000000012206c6f0
}
(lldb) gu
(lldb) call args
(volatile PLyTypeInfo *) $62 = 0x00007fff5912fbe8
(lldb) call *args
(PLyTypeInfo) $63 = {
in = {
d = {
func = 0x00000000218f8ed0
typfunc = {
fn_addr = 0x00007fff5912ff08
fn_oid = 1494417744
fn_nargs = 32767
fn_strict = '\0'
fn_retset = '\0'
fn_stats = '0'
fn_extra = 0x00007fca2206d060
fn_mcxt = 0x00007fca22800960
fn_expr = 0x00007fff5912fbe8
}
typtransform = {
fn_addr = 0x00007fca218f8e38
fn_oid = 570849696
fn_nargs = 32714
fn_strict = '\0'
fn_retset = '\0'
fn_stats = '8'
fn_extra = 0x00007fca22066f60
fn_mcxt = 0x0000000000000000
fn_expr = 0x00007fff5912fce0
}
typoid = 1494416304
typmod = 32767
typioparam = 114900880
typbyval = '\x01'
typlen = 0
typalign = '\xa6'
elm = 0x000000010735c8a0
}
r = {
atts = 0x00000000218f8ed0
natts = 1494417160
}
}
out = {
d = {
func = 0x00007fff5912ff08
typfunc = {
fn_addr = 0x000000010786ac18 (plpython3.so`PLy_CSreceive + 360
at plpy_spi.c:455)
fn_oid = 570846544
fn_nargs = 32714
fn_strict = '\0'
fn_retset = '\0'
fn_stats = '\x80'
fn_extra = 0x000000005912fce0
fn_mcxt = 0x0000000106d4d437
fn_expr = 0x00007fca2206ce30
}
typtransform = {
fn_addr = 0x00007fca22062f90
fn_oid = 0
fn_nargs = 0
fn_strict = '\0'
fn_retset = '\0'
fn_stats = '\0'
fn_extra = 0xffffffdc22066c20
fn_mcxt = 0x0000000000000000
fn_expr = 0x00007fca22066f60
}
typoid = 3756919206
typmod = -754934605
typioparam = 1494416704
typbyval = '\xff'
typlen = 0
typalign = '^'
elm = 0x00007fff5912fd10
}
r = {
atts = 0x00007fff5912ff08
natts = 126266392
}
}
is_rowtype = 0
typ_relid = 0
typrel_xmin = 570847072
typrel_tid = {
ip_blkid = (bi_hi = 32714, bi_lo = 0)
ip_posid = 36408
}
mcxt = 0x000000012206c6f0
}
(lldb)

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)