Read access violation on PG 9.5 (MSVC x64 build)

[mlt]

Hi! I'm new to temporal_tables and it happened such that I already upgraded my PG to 9.5 that is not mentioned explicitly in README as supported. Somehow modified project builds fine with MSBuild but fails to load into MSVS 2015 IDE. So I created a new project for debugging

Anyway, I followed the example steps, got backend pid with SELECT pg_backend_pid(); , attached my Visual Studio debugger to it, then when attempting to INSERT data I'm getting

Exception thrown: read access violation.
list_head(...) returned nullptr.

Here is the call stack

>   temporal_tables.dll!get_current_temporal_context(bool will_modify) Line 165 C
    temporal_tables.dll!get_system_time(...) Line 300   C
    temporal_tables.dll!versioning_insert(TriggerData * trigdata, TypeCacheEntry * typcache, int period_attnum) Line 874    C
    temporal_tables.dll!versioning(FunctionCallInfoData * fcinfo) Line 258  C
    postgres.exe!ExecCallTriggerFunc(TriggerData * trigdata, int tgindx, FmgrInfo * finfo, Instrumentation * instr, MemoryContextData * per_tuple_context) Line 1917    C
    postgres.exe!ExecBRInsertTriggers(EState * estate, ResultRelInfo * relinfo, TupleTableSlot * slot) Line 2042    C
    postgres.exe!ExecInsert(ModifyTableState * mtstate, TupleTableSlot * slot, TupleTableSlot * planSlot, List * arbiterIndexes, OnConflictAction onconflict, EState * estate, char canSetTag) Line 272 C
    postgres.exe!ExecModifyTable(ModifyTableState * node) Line 1442 C
    postgres.exe!ExecProcNode(PlanState * node) Line 389    C
    postgres.exe!ExecutePlan(EState * estate, PlanState * planstate, CmdType operation, char sendTuples, long numberTuples, ScanDirection direction, _DestReceiver * dest) Line 1555    C
    postgres.exe!standard_ExecutorRun(QueryDesc * queryDesc, ScanDirection direction, long count) Line 348  C
    postgres.exe!ProcessQuery(PlannedStmt * plan, const char * sourceText, ParamListInfoData * params, _DestReceiver * dest, char * completionTag) Line 190 C
    postgres.exe!PortalRunMulti(PortalData * portal, char isTopLevel, _DestReceiver * dest, _DestReceiver * altdest, char * completionTag) Line 1289    C
    postgres.exe!PortalRun(PortalData * portal, long count, char isTopLevel, _DestReceiver * dest, _DestReceiver * altdest, char * completionTag) Line 816  C
    postgres.exe!exec_simple_query(const char * query_string) Line 1111 C
    postgres.exe!PostgresMain(int argc, char * * argv, const char * dbname, const char * username) Line 4032    C
    postgres.exe!BackendRun(Port * port) Line 4238  C
    postgres.exe!SubPostmasterMain(int argc, char * * argv) Line 4729   C
    postgres.exe!main(int argc, char * * argv) Line 211 C
    [External Code] 

_temporalcontexts is NULL on that line. Any idea what can I do next? P.S. I tried later with 9.4 64bit, it also fails on INSERT. Am I missing something? P.P.S. I'm using stock builds from EnterpriseDB on Windows 7 64 bit. PPPS It looks like _PG_init() should have initialized temporal_contexts. Does something here rely on fork() and a copy of process' memory into children?

[mlt]

I installed Microsoft Child Process Debugging Power Tool for the sake of convenience to monitor all workers, but no break point was triggered at _PG_init. So I instrumented _PG_init with a call elog(ERROR, "temporal_contexts list created at 0x%x", temporal_contexts); and somehow I don't see anything in the console. Similar call at get_current_temporal_context dumps 0 into a console.

[mlt]

Uhm... are we missing Module Definitions File? or __declspec(dllexport)? Perhaps PGDLLEXPORT for _PG_init is necessary.

[mlt]

Bingo! Expect PR with 3 commits.

[arkhipov]

Hello @mlt,

Thank you for fixing the problem. The last commit in the master branch is a bit experimental. I have not tested it much (and I have not tested it at all in Windows).

Also many thanks for the hint about AppVeyor. I will try to set up a build worker with regression tests at the weekend.

[mlt]

Not a problem! I guess I'm the first Windows user :sunglasses: I just re-read your comment, I see that _PG_init wasn't there before that commit. Indeed, I didn't try messing with time setting, so I can't comment on that, but the very essential part worked fine for me on 9.5 64 bit. I don't know how it is done on Travis (never used) but one can cache files, and use curl to download extra software (9.5) if absent and install it before the build. I'd try with AppVeyor web GUI first before diving into appveyor.yml. It defaults to your cloned repo folder, so

if not exist %PG95% curl -sLO http://get.enterprisedb.com/postgresql/%PG95%
%PG95% --unattendedmodeui minimal --mode unattended --superpassword "password" --servicename "postgreSQL-9.5" --servicepassword "password" --serverport 5435

where PG95 is an environment variable set to postgresql-9.5.0-1-windows-x64.exe, and add cache entry for postgresql-9.5.0-1-windows-x64.exe. It probably shouldn't hurt installing it even for other builds, AppVeyor build worker machines are pretty fast, and with 2 files to compile, it shouldn't take excessive time. There are silent install keys documented here. Alternatively, I can update my PR to whatever is necessary and include appveyor.yml , probably, along with tests, and transfer AppVeyor account by using e-mail address from your commits and send you a temporary password. You may send me you PGP public key if you prefer it that way. P.S. I'd add RelWithDebInfo configuration akin to what CMake generates so there are (PDB) symbols like for PG itself. P.P.S. There is also a catch, that your project is for MSVC 2015, while EDB builds use MSVC 2013 if I'm not mistaken. I didn't check whether they pull dynamic CRT, otherwise it would be a mess for end users to have to install 2015 runtime.

[mlt]

I do confirm postgres.exe is linked against msvcr120.dll, while your project introduces vcruntime140.dll dependency. AFAIK mixing isn't a good thing to do. But it might worth a shot to build with static CRT. What are your thoughts on that? I personally would suggest CMake and generating MSVC specific projects, but CMake is quite heavy for 2 C files. I dunno whether msbuild can use newer project to build for previous platform... they all are installed on AppVeyor. I might have found an answer http://stackoverflow.com/a/20321253/673826

[mlt]

I've updated PR (needs autosquish though) and I've added you to "collaborators" in AppVeyor. You should be able to access it with your GitHub account. Once we are done with everything, you can export appveyor.yml into your repo, and recreate an AppVeyor project so I can delete mine. It is a PITA to deal with accounts on AppVeyor. AFAIK it is close to impossible to transfer an account without tech support involvement.

[arkhipov]

Probably I am missing something, but I cannot get access to the project in AppVeyor (though I have received a message that I was added as collaborator). I can only see the build history and logs.

I played a bit with the build last weekend. I am still unsure how to add the regression tests to the build. Do you have any thoughts?

[arkhipov]

OK. I think I got it now. I had to re-login using another account on AppVeyor.

[mlt]

I thought I replied by e-mail, but somehow it didn't make it here :open_mouth: So I copy it below with some edits.

Hi! Since you logged in. There is a Tests tab in settings. I sed & grep tests. It is not perfect but so far it seems ok. However I'm not certain how to make PG echo complete commands through psql. You probably see what I mean like CREATE TABLE _actual_table_name_from_expected_ismissing.

I ended up using full echo for psql, but now those server responses hang unmatched as can be seen here.

I'm not very familiar how regression testing works with PGXN on *nix. Does it still use psql with some options?

[mlt]

I think I finalized scripting tests and it looks acceptable.

[arkhipov]

The *nix Makefile uses pg_regress to run tests. There should be pg_regress.exe in bin directory.

pg_regress --inputdir=./ --psqldir='/usr/lib/postgresql/9.4/bin' --dbname=contrib_regression test1 test2 test3

I am not a Windows expert, so I am not familiar with best practices for testing in Msbuild. Is it possible to make them part of the build process, not something that is done in AppVeyor only?

[mlt]

There should be pg_regress.exe in bin directory

Never noticed that. I'm surprised web search doesn't yield much official docs. And I'm surprised no-one suggested that on IRC.

Is it possible to make them part of the build process

The closest thing that comes to mind is to add Exec Task.

not something that is done in AppVeyor only

Are you fine maintaining two places enumerating tests? sed I used comes from Git installation that happened to be on PATH on AppVeyor builder machines. We can reasonably assume that everyone trying to build this extension got it with Git. I mean we could fetch the list of tests as I had before with sed.

If you don't like sed approach, another option is to add extra target to Makefile to echo tests list. One way is to have a "custom" _pgconfig that would return some local file when invoked with --pgxs with target all that echos $(REGRESS) and this can be fed via PowerShell into pg_regress.exe .

P.S. That was brutal to wipe most PowerShell code instead of commenting it out. I didn't have a copy :innocent: P.P.S. I'm about to fix invocation as it is right now.

[arkhipov]

I have fixed the build; however, some tests failed due to low system clock resolution. So I needed some time to fix them as well. The branch is here.

Sorry for removing your code. Here is the original tests script.

[mlt]

I didn't realize until recently that you might be changing things at the same time... There is another "problem": 9.3 binary from EnterpriseDB is built with Visual Studio 2010, i.e. v100, that somehow fails. Also 9.5 has no --psqldir. So we either skip that for that version or always use pg_regress from 9.4.

[mlt]

I'm done messing with things. There is another commit I've added into Pull Request for missing nextafter in VS 2010 introduced in VS 2013. Update: oops... pg_regress from 9.5 is really broken Update: done now. 9.5 does pass tests as they were.

[arkhipov]

I think I have it working now. Could you please review the AppVeyor configuration and the branch?

I switched to cmd because I have not found a way to make PowerShell not to consider the stderr output that pg_regress prints while testing as an error.

[mlt]

Your branch looks good to me :+1: I don't know PowerShell myself much either. It is supposedly easier for things automation due to extensibility, but it might be an overkill in this case. It looks like 2> Out-Null might have made a trick. Also, I used ldd primarily to check msvc runtime only. I found out for myself that one can use findstr in place of grep on Windows. So if you think ldd outputs too much stuff, perhaps | findstr msvcr should reduce it and still indicate correct runtime library linkage. Or comment out ldd whatsoever since we know those are correct platform toolsets. Or just leave it as is and merge --no-ff into master :sunglasses:

[mlt]

On a side note, I don't know if it worth pushing further and adding PDB (debug symbol) generation alike PG main stuff. IIRC line numbering might get lost with whole program link optimization (/GL). But I think as long as it links and passes tests, there might be no need for symbols much on Windows.

[mlt]

Oh... and about YML. There are alternatives how to write commands. The way it is done now with - cmd: >- syntax, you are out of multiline commands, i.e. can't split long lines. One could use usual - something instead of a block syntax for each new command. I personally feel like lots of vertical space is lost with - cmd: >- blocks. I mean something like this.

And probably you could wipe out traces of VisualStudioVersion. As platform toolset Windows7.1SDK does the job for 9.3

[arkhipov]

I have merged the branch into master. Thank you for your cooperation!