Re: [HACKERS] Odd behavior with PG_TRY - Mailing list pgsql-hackers
From | Jim Nasby |
---|---|
Subject | Re: [HACKERS] Odd behavior with PG_TRY |
Date | |
Msg-id | 3875fccc-178f-51b5-87a2-dddede565aa7@BlueTreble.com Whole thread Raw |
In response to | Re: [HACKERS] Odd behavior with PG_TRY (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: [HACKERS] Odd behavior with PG_TRY
|
List | pgsql-hackers |
On 1/2/17 9:47 PM, Tom Lane wrote: > Jim Nasby <Jim.Nasby@BlueTreble.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)
pgsql-hackers by date: