kokizzu / plv8js

Automatically exported from code.google.com/p/plv8js
Other
0 stars 0 forks source link

create function causes PostgreSQL session to crash on windows if typo in stored func #29

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. CREATE OR REPLACE FUNCTION upsert_inventory(param_inv json) RETURNS
text AS $$
var o = JSON.parse(param_inv);
var uplan = plv8.prepare('UPDATE inventory 
    SET prod_name = $1, loc_id = $2 WHERE prod_code = $3', ['varchar', 'int', 'varchar'] );
var iplan = plv8.prepare('INSERT INTO inventory( prod_name, loc_id, prod_code)  
    VALUES($1, $2, $3', ['varchar', 'int', 'varchar'] );
var num_changed;
if (typeof ej != 'object')
   return NULL;
else {
    for(var i=0; i<o.length; i++){
      num_changed = uplan.execute(o[i]['prod_name'], o[i]['loc_id'], o[i]['prod_code']);
      if (num_changed == 0){ /** record does not exist add it **/
        iplan.execute(o[i]['prod_name'], o[i]['loc_id'], o[i]['prod_code']);
      }
    }
}
return 'done';
$$ LANGUAGE plv8 VOLATILE;

What is the expected output? Hmm it gives an error.

What do you see instead? Crashes my session.
NOTE:  If I change the statement to this it doesn't crash and saves fine.  The 
difference between the two is I have line breaks in my SQL statements in the 
crashing ones.
CREATE OR REPLACE FUNCTION upsert_inventory(param_inv json) RETURNS
text AS $$
var o = JSON.parse(param_inv);
var uplan = plv8.prepare('UPDATE inventory SET prod_name = $1, loc_id = $2 
WHERE prod_code = $3', ['varchar', 'int', 'varchar'] );
var iplan = plv8.prepare('INSERT INTO inventory( prod_name, loc_id, prod_code) 
VALUES($1, $2, $3', ['varchar', 'int', 'varchar'] );
var num_changed;
if (typeof ej != 'object')
   return NULL;
else {
    for(var i=0; i<o.length; i++){
      num_changed = uplan.execute(o[i]['prod_name'], o[i]['loc_id'], o[i]['prod_code']);
      if (num_changed == 0){ /** record does not exist add it **/
        iplan.execute(o[i]['prod_name'], o[i]['loc_id'], o[i]['prod_code']);
      }
    }
}
return 'done';
$$ LANGUAGE plv8 VOLATILE;

What version of the product are you using? On what operating system?
I compiled under mingw64 gcc 4.5.4.  I tested under both my mingw64 PostgreSQL 
build and the EDB PostgreSQL 9.2beta2 builds and both give the same problem.

Please provide any additional information below.

PostgreSQL 9.2beta2
Windows 7 64-bit (I tested on 32-bit EDB and 64-bit mingw built PostgreSQL 
(64-bit) )

The postgres logs show:

ERROR:  column "spclocation" does not exist at character 25
STATEMENT:  SELECT ts.oid, spcname, spclocation, spcoptions, 
pg_get_userbyid(spcowner) as spcuser, spcacl, pg_catalog.shobj_description(oid, 
'pg_tablespace') AS description FROM pg_tablespace ts
     ORDER BY spcname
terminate called after throwing an instance of 'js_error';

Using V8 compiled from trunk (Jul 15 2012 or so) and PL/V8 from git around Jul 
15 2012

I assume its something with the parser.  Note to compile plv8 I had to change 
SHLIB_LINK to (added stdc++)
SHLIB_LINK := $(SHLIB_LINK) -lv8 -lstdc++ 

as described
http://www.postgresonline.com/journal/archives/261-Building-PLV8JS-and-PLCoffee-
for-Windows-using-MingW64-w64-w32.html

so not sure if that is part of the issue.  I did leave setting to compile with 
g++ though.

Original issue reported on code.google.com by reg...@arrival3d.com on 21 Jul 2012 at 3:16

GoogleCodeExporter commented 9 years ago
Disregard the spclocation does not exist error.  I realize that was caused 
because I launched my pgAdmin 9.1 instead of 9.2.

So only error really is:
terminate called after throwing an instance of 'js_error'
LOG:  server process (PID 4812) exited with exit code 3
LOG:  terminating any other active server processes

Original comment by reg...@arrival3d.com on 21 Jul 2012 at 3:37

GoogleCodeExporter commented 9 years ago
Not sure its also relevant, but when building plv8, I did get an error that d8 
does not exist.  Do I need to compile d8 maybe for things to work properly?

Original comment by reg...@arrival3d.com on 21 Jul 2012 at 3:45

GoogleCodeExporter commented 9 years ago
Okay I compiled d8, but that didn't help aside from getting passed the d8 is 
not found notice.

It seems any malformed function I create will trigger the issue For example:

CREATE OR REPLACE FUNCTION test_blow_up() RETURNS
text AS $$
a.
$$ LANGUAGE plv8 VOLATILE;

Original comment by reg...@arrival3d.com on 21 Jul 2012 at 7:34

GoogleCodeExporter commented 9 years ago
The d8 issue shouldn't affect anything. I have a fix for it in any case.

Original comment by AMDuns...@gmail.com on 21 Jul 2012 at 8:50

GoogleCodeExporter commented 9 years ago
It sounds bizarre.  The symptom looks uncaught exception happens, although 
js_error is caught properly in mac/linux.  Reading the compile message above, 
__cxa_throw is from gcc tool chain, and PostgreSQL is built with VC, something 
may happen.  As we are not window/mingw *and* c++ expert, it may take time to 
solve it.  I hope we have someone who is good at both of them, but anyway I'll 
try to look at it in windows.

Original comment by umi.tan...@gmail.com on 22 Jul 2012 at 10:35

GoogleCodeExporter commented 9 years ago
It happens on my mingw built PostgresQL as well though so don't think it has to 
do with VC unless my visual 2010 install on my pc crept in somewhere when I was 
compiling my mingw.

Are  you building on Mac with Scones or GYP.  My next thought was to try to 
build V8 under GYP with VC 2010.

The _cxa_throw error is what I get during compile of plv8 if I don't add the 

-lstdc++

so wonder if those are related.

Original comment by reg...@arrival3d.com on 23 Jul 2012 at 1:49

GoogleCodeExporter commented 9 years ago
Not sure if this is related, I don't have: custom_variable_classes set.
I don't seem to be able to set this in 9.2beta2 postgresql.conf  get
"unrecognized configuration parameter "custom_variable_classes"

installcheck gives this output: (though I understand I should be running this 
with custom_variable_classes='plv8')

NOTICE:  database "contrib_regression" does not exist, skipping
DROP DATABASE
============== creating database "contrib_regression" ==============
CREATE DATABASE
ALTER DATABASE
============== running regression test queries        ==============
test init-extension           ... ok
test plv8                     ... FAILED (test process exited with exit code 2)
test inline                   ... ok
test json                     ... ok
test startup_pre              ... ok
test startup                  ... FAILED (test process exited with exit code 2)
test varparam                 ... FAILED (test process exited with exit code 2)

Original comment by reg...@arrival3d.com on 23 Jul 2012 at 7:25

GoogleCodeExporter commented 9 years ago
custom_variable_classes is deprecated in 9.2.  It doesn't matter.  The test 
result is expected as all exception handling is not working.  I'm trying 
install postgres binary in EC2's windows server, but couldn't so far.  Where 
can I get windows server to try it...

Original comment by umi.tan...@gmail.com on 23 Jul 2012 at 7:28

GoogleCodeExporter commented 9 years ago
What error are you getting trying to install binary?  If you are using the 
PostgreSQL 9.2 beta2 binaries from here:
http://www.enterprisedb.com/products-services-training/pgbindownload

You need to also install the VC 2010 Runtime.
http://www.microsoft.com/en-us/download/details.aspx?id=5555

Anyrate I get teh same error under pure ming64.  I can package up my tarball 
and it should just work if you extract it.  Give me a minute to do that.

Original comment by reg...@arrival3d.com on 23 Jul 2012 at 8:39

GoogleCodeExporter commented 9 years ago
Actually is your windows server 32bit or 64-bit.  I have both ming32 and ming64.

Original comment by reg...@arrival3d.com on 23 Jul 2012 at 8:40

GoogleCodeExporter commented 9 years ago
windows installer issue was solved.  Instead of mingw, I tried to build v8 with 
VC express, and still no good news.

Original comment by umi.tan...@gmail.com on 23 Jul 2012 at 8:43

GoogleCodeExporter commented 9 years ago
Can you try to build a4ee634ce599cc5eaeb9605067eb6b5eae6e42d5 and run 
installcheck?  That version was built with mingw, I guess.  If it passes, 
something in between was broken.

Original comment by umi.tan...@gmail.com on 23 Jul 2012 at 9:12

GoogleCodeExporter commented 9 years ago
That one I can't compile at all. Gives error:

In file included from plv8.cc:4:0:
plv8.h:126:61: error: macro "Yield" passed 1 arguments, but takes just 0
plv8.h:126:63: error: expected initializer before 'throw'
make: *** [plv8.o] Error 1

Original comment by reg...@arrival3d.com on 23 Jul 2012 at 11:32

GoogleCodeExporter commented 9 years ago
Sounds strange to me.  Can you try to rename Yield?  It's definitely a function.

diff --git a/plv8.h b/plv8.h
index b5322bc..73cd43a 100644
--- a/plv8.h
+++ b/plv8.h
@@ -123,7 +123,7 @@ extern v8::Handle<v8::Value> FreePlan(const v8::Arguments& 
args)
 extern v8::Handle<v8::Value> CreateCursor(const v8::Arguments& args) throw();
 extern v8::Handle<v8::Value> FetchCursor(const v8::Arguments& args) throw();
 extern v8::Handle<v8::Value> CloseCursor(const v8::Arguments& arg) throw();
-extern v8::Handle<v8::Value> Yield(const v8::Arguments& args) throw();
+extern v8::Handle<v8::Value> Yield_foo(const v8::Arguments& args) throw();
 extern v8::Handle<v8::Function> CreateYieldFunction(Converter *conv, Tuplestorestat

 #endif // _PLV8_
diff --git a/plv8_func.cc b/plv8_func.cc
index 42bcc54..964a058 100644
--- a/plv8_func.cc
+++ b/plv8_func.cc
@@ -451,7 +451,7 @@ CloseCursorInternal(const Arguments& args)
 }

 Handle<v8::Value>
-Yield(const Arguments& args) throw()
+Yield_foo(const Arguments& args) throw()
 {
        return SafeCall(YieldInternal, args);
 }
@@ -476,5 +476,5 @@ CreateYieldFunction(Converter *conv, Tuplestorestate 
*tupstore)
        Handle<Array> data = Array::New(2);
        data->Set(0, External::Wrap(conv));
        data->Set(1, External::Wrap(tupstore));
-       return FunctionTemplate::New(Yield, data)->GetFunction();
+       return FunctionTemplate::New(Yield_foo, data)->GetFunction();
 }

Original comment by umi.tan...@gmail.com on 23 Jul 2012 at 4:23

GoogleCodeExporter commented 9 years ago
Unfortunately that got rid of the Yield error but then a whole bunch of other 
errors showed their ugly head.  I even tried going back to an earlier V8.  The 
3.6.2 noted in your README and that one I can't even get to compile under 
MingW.  I thin the fixes they made to V8 to get it to compile under mingw64 
came after that.

Original comment by reg...@arrival3d.com on 24 Jul 2012 at 4:28

GoogleCodeExporter commented 9 years ago
I'm not sure if these two issues are related, but I have the same issue with 
GEOS. Works great except for the tests where it tries to throw an exception.

Except that one rears its ugly head only if I compile with GCC higher than 4.5 
which is why I've stuck with 4.5 

http://trac.osgeo.org/geos/ticket/528

GEOS is a ix of C and C++ (has a C interface to a C++ underlying infrastructure)

Original comment by reg...@arrival3d.com on 24 Jul 2012 at 4:32

GoogleCodeExporter commented 9 years ago
Yeah, it sounds like mingw's issue. The exception mechanism doesn't work 
correctly for some reason.

Original comment by umi.tan...@gmail.com on 24 Jul 2012 at 5:22

GoogleCodeExporter commented 9 years ago
Well I don't think it's just mingw though.  If you see the ticket, I have the 
same issue compiling under VC++ SDK 64-bit.  I had thought like you it was a 
mingw issue initially.

Original comment by reg...@arrival3d.com on 24 Jul 2012 at 6:30

GoogleCodeExporter commented 9 years ago
I heard a lot of these issues go away if I statically link in libstdc++.  
Actually the stdc++ I was getting without -lstdc++ doesn't happen until the 
dllwrap phase.

I'm not having much luck with statically linking my libstdc++ to try this 
theory out.

Was trying something like this:
g++ -static-libstdc++

basically trying ot manyallu run the generated g++ commands but with 
-static-libstdc++ in there.

Original comment by reg...@arrival3d.com on 24 Jul 2012 at 9:09

GoogleCodeExporter commented 9 years ago
Interesting.  Does this help?

diff --git a/Makefile b/Makefile
index f03c0df..4456cf5 100644
--- a/Makefile
+++ b/Makefile
@@ -14,7 +14,7 @@ EXTVER = 1.1.0
 DATA = plv8.control plv8--$(EXTVER).sql
 DATA_built = plv8.sql
 REGRESS = init-extension plv8 inline json startup_pre startup varparam
-SHLIB_LINK := $(SHLIB_LINK) -lv8
+SHLIB_LINK := $(SHLIB_LINK) -lv8 -static-libstdc++

 META_VER := $(shell v8 -e 'print(JSON.parse(read("META.json")).version)' 2>/dev/nul
 ifndef META_VER

Original comment by umi.tan...@gmail.com on 25 Jul 2012 at 10:34

GoogleCodeExporter commented 9 years ago
No gives unrecognized option -static-libstdc++

I know with a mingw64 runing 4.6.1 I was experimenting with (before I ran into 
my GEOS woes), it did automatically compile in libstdc++ though that didn't 
help the GEOS > 4.5 issues I was having.

Original comment by reg...@arrival3d.com on 25 Jul 2012 at 11:47

GoogleCodeExporter commented 9 years ago
Yeh I got all tests to pass under mingw with help from a comment on this link:
http://www.trilithium.com/johan/2005/06/static-libstdc/

anyway by doing this:

CUSTOM_CC = gcc
SHLIB_LINK := $(SHLIB_LINK) -lv8 -Wl,-Bstatic -lstdc++ -Wl,-Bdynamic -lm

/** As I recall I think your code was hard-coded to use g++ here **/
%.o : %.cc
    ${CUSTOM_CC}  $(CCFLAGS) $(CPPFLAGS) -I $(V8DIR)/include -fPIC -c -o $@ $<

I'll try next under my VC++ install to see if it works.

Original comment by reg...@arrival3d.com on 25 Jul 2012 at 12:37

GoogleCodeExporter commented 9 years ago
Cool the library I built under mingw works on EDB 9.2beta2 as well.
I still need to run the same exercise on my 64-bit install (this is just my 
32-bit)

Now when I run:
CREATE OR REPLACE FUNCTION test_blow_up() RETURNS
text AS $$
a.
$$ LANGUAGE plv8 VOLATILE;

I get expected:
ERROR:  SyntaxError: Unexpected token }
DETAIL:  test_blow_up() LINE 4: })

The plv8.dll is much bigger now (4MB though I haven't stripped it yet) and has 
no dependencies except v8.dll.  I guess it compiled in both libgcc and lstd.

Not sure if licensing issues packaging along a statically compiled in libstdc++ 
though my v8.dll I think is still dynamic so I have to distribute those anyway.

Original comment by reg...@arrival3d.com on 25 Jul 2012 at 12:47

GoogleCodeExporter commented 9 years ago
Same trick works for 64-bit so my mingw64-w64 compiled plv8 works both on my 
mingw64 compiled PostgreSQL and the PostgreSQL 64-bit EDB 9.2beta2 binaries I 
downloaded.

Original comment by reg...@arrival3d.com on 25 Jul 2012 at 5:48

GoogleCodeExporter commented 9 years ago
So what's the suggested fix for the master?  If the platform is mingw/win, then 
CUSTOM_CC=gcc and SHLIB_LINK += -Wl,-Bstatic -lstdc++ -Wl,-Bdynamic -lm?

Original comment by umi.tan...@gmail.com on 26 Jul 2012 at 8:40

GoogleCodeExporter commented 9 years ago
I would be tempted to just put in a README for now. This is compiling under 
mingw64 chain.  The standard msys mingw may behave differently.
I updated my instructions here already to reflect this:

http://www.postgresonline.com/journal/archives/261-Building-PLV8JS-and-PLCoffee-
for-Windows-using-MingW64-w64-w32.html

I'm also not quite sure it's only an issue for MinGW. It may just be more 
apparent since so many versions of C++ compilers are used within the same app 
on windows many of which can't be controlled because they are dependency 
libraries in system32/syswow64.

I think this problem may happen anytime the libstd++ that plv8 is compiled with 
is different from any other components in use and these are then called from a 
C app. E.g v8 will need to be compiled with same version of g++ (becauseof 
C-ABI differences) (which is plausible except for case when you are relying on 
the platform to have v8).

I'm still puzzled why statically linking with -static-libstdc++ doesn't work 
when I'm compiling with g++ 4.5.4 might be something I have misconfigured or I 
need to upgrade.

Do you have a mingw to test with?  If not I can experiment with a newer one.

The other thing I'm puzzled about is why I didn't need to statically compile 
v8.dll.  Might be because it is pure c++ and doesn't use any of the try catch 
helpers in the libstdc++.  If I look with Dependency Walker, the v8.dll doesn't 
have any of those try catch calls in libstdc++ (where as plv8.dll uses all 
these __cxa_throw etc in libstdc++)

v8.dll, v8preparser on the other hand uses: _cxa_guard_acquire, 
__cxa_guard_release, __cxa_pure_virtual (some weird names: _ZdaPv, _ZdlPv, 
_Znaj,_Znwj)

Original comment by reg...@arrival3d.com on 26 Jul 2012 at 12:14

GoogleCodeExporter commented 9 years ago
v8 doesn't use try/catch at all, partly because they implicitly use the stack 
frame for the efficient garbage collection AFAIK.  So the choice of try/catch 
is plv8's.  In theory different version of gcc tool chain may cause ABI 
difference at any time, but in this case it seems only try/catch was the 
problem.

Original comment by umi.tan...@gmail.com on 27 Jul 2012 at 6:02

GoogleCodeExporter commented 9 years ago
The master branch now allows you to build statically-linked version of plv8.  
Try `make static`, maybe you need to run VC to build the static libs under the 
downloaded directory, but after that you should be able to build it smoothly.  
Can this solve the issue here?

Original comment by umi.tan...@gmail.com on 2 Dec 2012 at 11:16

GoogleCodeExporter commented 9 years ago
okay will give a try next I have the chance. Hasn't been an issue for me since 
the

CUSTOM_CC = gcc
SHLIB_LINK := $(SHLIB_LINK) -lv8 -Wl,-Bstatic -lstdc++ -Wl,-Bdynamic -lm

hack worked pretty well for my case.   

Original comment by lr1234...@gmail.com on 2 Dec 2012 at 11:27

GoogleCodeExporter commented 9 years ago
Added these lines to README.
75ca373

Original comment by umi.tan...@gmail.com on 4 Dec 2012 at 6:45

GoogleCodeExporter commented 9 years ago
I have tried following this instruction on referenced article 
(http://www.postgresonline.com/journal/archives/261-Building-PLV8JS-and-PLCoffee
-for-Windows-using-MingW64-w64-w32.html) and this Issue, without success.

dependency walker reports strange missing dependencies as shwon:

https://github.com/hernad/plv8_build#install-plv8-binaries-mingw-compiled-postgr
esonline

Original comment by ernad.husremovic on 6 Dec 2012 at 9:55

GoogleCodeExporter commented 9 years ago
Ernad,

Are you building under the regular mingw or mingw64.  I've only done this on 
mingw64-w32, mingw64-w64 chains.  Regarding those dependency walkers you have I 
think you can ignore them.  I think I get those too and its just because your 
search path of dependency walker doesn't include where those are located.  As I 
recall I se those same missing in all my builds of anything.

I also try to stay away from gcc 4.6.1, for building postgis for example our 
GEOS dependency crashes badly under gcc 4.6.1 so I think I'm running gcc 4.5 
something.

If you want, you can download the mingw64-w32 chain I packaged a couple months 
ago for building postgis which is pretty much same I use to build v8 etc and 
try to work with that.

http://www.bostongis.com/postgisstuff/ming32.zip

Original comment by reg...@arrival3d.com on 6 Dec 2012 at 12:14

GoogleCodeExporter commented 9 years ago
@Reg..

Thank you for ming32.zip

It took me two days to put it all together, but I have succeded at the end :)

https://github.com/hernad/plv8_build/blob/master/README.md

May I add the link to your min32 stuff in my README:

> http://www.bostongis.com/postgisstuff/ming32.zip

Original comment by ernad.husremovic on 8 Dec 2012 at 7:56

GoogleCodeExporter commented 9 years ago
Sure you can add the link.

Original comment by reg...@arrival3d.com on 8 Dec 2012 at 10:12

GoogleCodeExporter commented 9 years ago
As a side note, I have a similar zip for building the PostgreSQL 64-bit windows 
version.  Feel free to include that link as well.

http://www.bostongis.com/postgisstuff/ming64.zip

Original comment by reg...@arrival3d.com on 9 Dec 2012 at 12:46

GoogleCodeExporter commented 9 years ago
@Regina, links are included 

https://github.com/hernad/plv8_build#prerequisites

Thank you again.

Original comment by ernad.husremovic on 10 Dec 2012 at 8:20