plpython issue with Win64 (PG 9.2)

Started by Asif Naeemover 13 years ago10 messages

asif.naeem@enterprisedb.com

over 13 years ago

Hi,

On Windows 7 64bit, plpython is causing server crash with the following
test case i.e.

CREATE PROCEDURAL LANGUAGE 'plpython3u';

CREATE OR REPLACE FUNCTION pymax (a integer, b integer)
RETURNS integer
AS $$
if a > b:
return a
return b
$$ LANGUAGE plpython3u;
SELECT pymax(1, 2);

Server exit with the following exception i.e.

Unhandled exception at 0x777a3483 in postgres.exe: 0xC00000FD: Stack
overflow.

plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int *
tb_depth) Line 174 C
plpython3.dll!PLy_elog(int elevel, const char * fmt, ...) Line 67
C
plpython3.dll!PLyUnicode_AsString(_object * unicode) Line 96 C
plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int *
tb_depth) Line 176 + 0x8 bytes C
plpython3.dll!PLy_elog(int elevel, const char * fmt, ...) Line 67
C
plpython3.dll!PLyUnicode_AsString(_object * unicode) Line 96 C
...
...
plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int *
tb_depth) Line 176 + 0x8 bytes C
plpython3.dll!PLy_elog(int elevel, const char * fmt, ...) Line 67
C
plpython3.dll!PLyUnicode_AsString(_object * unicode) Line 96 C
plpython3.dll!PLy_traceback(char * * xmsg, char * * tbmsg, int *
tb_depth) Line 176 + 0x8 bytes C

Dbserver get stuck in the following call loop i.e.
... PLy_elog() -> PLy_traceback() -> PLyUnicode_AsString() ->
PLyUnicode_Bytes() -> PLy_elog() ...

I think primary reason that trigger this issue is when Function
PLyUnicode_Bytes() calls "PyUnicode_AsEncodedString( ,WIN1252 /*Server
encoding*/, ) " it fails with null. I built latest pg 9.2 source code with
python 3.2.2.3 by using Visual Studio 2010. Thanks.

Best Regards,
Muhammad Asif Naeem

Jan Urbański

wulczer@wulczer.org

over 13 years ago

In reply to: Asif Naeem (#1)

Re: plpython issue with Win64 (PG 9.2)

On 27/06/12 11:51, Asif Naeem wrote:

Hi,

On Windows 7 64bit, plpython is causing server crash with the following
test case i.e.

CREATE PROCEDURAL LANGUAGE 'plpython3u';

CREATE OR REPLACE FUNCTION pymax (a integer, b integer)
RETURNS integer
AS $$
if a> b:
return a
return b
$$ LANGUAGE plpython3u;
SELECT pymax(1, 2);

I think primary reason that trigger this issue is when Function
PLyUnicode_Bytes() calls "PyUnicode_AsEncodedString( ,WIN1252 /*Server
encoding*/, ) " it fails with null. I built latest pg 9.2 source code with
python 3.2.2.3 by using Visual Studio 2010. Thanks.

I'll try to reproduce this on Linux, which should be possible given the
results of your investigation.

Cheers,
Jan

Jan Urbański

wulczer@wulczer.org

over 13 years ago

In reply to: Jan Urbański (#2)

Re: plpython issue with Win64 (PG 9.2)

On 27/06/12 13:57, Jan Urbański wrote:

On 27/06/12 11:51, Asif Naeem wrote:

Hi,

On Windows 7 64bit, plpython is causing server crash with the following
test case i.e.

CREATE PROCEDURAL LANGUAGE 'plpython3u';

CREATE OR REPLACE FUNCTION pymax (a integer, b integer)
RETURNS integer
AS $$
if a> b:
return a
return b
$$ LANGUAGE plpython3u;
SELECT pymax(1, 2);

I think primary reason that trigger this issue is when Function
PLyUnicode_Bytes() calls "PyUnicode_AsEncodedString( ,WIN1252 /*Server
encoding*/, ) " it fails with null. I built latest pg 9.2 source code
with
python 3.2.2.3 by using Visual Studio 2010. Thanks.

I'll try to reproduce this on Linux, which should be possible given the
results of your investigation.

Your analysis is correct, I managed to reproduce this by injecting

serverenc = "win1252";

into PLyUnicode_Bytes. The comment in that function says that Python
understands all PostgreSQL encoding names except for SQL_ASCII, but
that's not really true. In your case GetDatabaseEncodingName() returns
"WIN1252" and Python accepts "CP125".

I'm wondering how this should be fixed. Just by adding more special
cases in PLyUnicode_Bytes?

Even if we add a switch statement that would convert PG_WIN1250 into
"CP1250", Python can still raise an exception when encoding (for various
reasons). How about replacing the PLy_elog there with just an elog? This
loses traceback support and the Python exception message, which could be
helpful for debugging (something like "invalid character <foo> for
encoding cp1250"). OTOH, I'm uneasy about invoking the entire PLy_elog
machinery from a function that's as low-level as PLyUnicode_Bytes.

Lastly, we map SQL_ASCII to "ascii" which is arguably wrong. The
function is supposed to return bytes in the server encoding, and under
SQL_ASCII that probably means we can return anything (ie. use any
encoding we deem useful). Using "ascii" as the Python codec name will
raise an error on anything that has the high bit set.

So: I'd add code to translate WINxxx into CPxxx when choosing the Python
to use, change PLy_elog to elog in PLyUnicode_Bytes and leave the
SQL_ASCII case alone, as there were no complaints and people using
SQL_ASCII are asking for it anyway.

Cheers,
Jan

Asif Naeem

asif.naeem@enterprisedb.com

over 13 years ago

In reply to: Jan Urbański (#3)

Re: plpython issue with Win64 (PG 9.2)

Thank you. Please do let me know once fix check-in. I will test it and
share feedback with you. Thanks.

Best Regards,
Asif Naeem

On Fri, Jun 29, 2012 at 3:36 AM, Jan Urbański <wulczer@wulczer.org> wrote:

Show quoted text

On 27/06/12 13:57, Jan Urbański wrote:

On 27/06/12 11:51, Asif Naeem wrote:

Hi,

On Windows 7 64bit, plpython is causing server crash with the following
test case i.e.

CREATE PROCEDURAL LANGUAGE 'plpython3u';

CREATE OR REPLACE FUNCTION pymax (a integer, b integer)
RETURNS integer
AS $$
if a> b:
return a
return b
$$ LANGUAGE plpython3u;
SELECT pymax(1, 2);

I think primary reason that trigger this issue is when Function
PLyUnicode_Bytes() calls "PyUnicode_AsEncodedString( ,WIN1252 /*Server
encoding*/, ) " it fails with null. I built latest pg 9.2 source code
with
python 3.2.2.3 by using Visual Studio 2010. Thanks.

I'll try to reproduce this on Linux, which should be possible given the
results of your investigation.

Your analysis is correct, I managed to reproduce this by injecting

serverenc = "win1252";

into PLyUnicode_Bytes. The comment in that function says that Python
understands all PostgreSQL encoding names except for SQL_ASCII, but that's
not really true. In your case GetDatabaseEncodingName() returns "WIN1252"
and Python accepts "CP125".

I'm wondering how this should be fixed. Just by adding more special cases
in PLyUnicode_Bytes?

Even if we add a switch statement that would convert PG_WIN1250 into
"CP1250", Python can still raise an exception when encoding (for various
reasons). How about replacing the PLy_elog there with just an elog? This
loses traceback support and the Python exception message, which could be
helpful for debugging (something like "invalid character <foo> for encoding
cp1250"). OTOH, I'm uneasy about invoking the entire PLy_elog machinery
from a function that's as low-level as PLyUnicode_Bytes.

Lastly, we map SQL_ASCII to "ascii" which is arguably wrong. The function
is supposed to return bytes in the server encoding, and under SQL_ASCII
that probably means we can return anything (ie. use any encoding we deem
useful). Using "ascii" as the Python codec name will raise an error on
anything that has the high bit set.

So: I'd add code to translate WINxxx into CPxxx when choosing the Python
to use, change PLy_elog to elog in PLyUnicode_Bytes and leave the SQL_ASCII
case alone, as there were no complaints and people using SQL_ASCII are
asking for it anyway.

Cheers,
Jan

Jan Urbański

wulczer@wulczer.org

over 13 years ago

In reply to: Jan Urbański (#3)

Re: plpython issue with Win64 (PG 9.2)

On 29/06/12 00:36, Jan Urbański wrote:

On 27/06/12 13:57, Jan Urbański wrote:

On 27/06/12 11:51, Asif Naeem wrote:

Hi,

On Windows 7 64bit, plpython is causing server crash with the following
test case i.e.

So: I'd add code to translate WINxxx into CPxxx when choosing the Python
to use, change PLy_elog to elog in PLyUnicode_Bytes and leave the
SQL_ASCII case alone, as there were no complaints and people using
SQL_ASCII are asking for it anyway.

Since no one commented, I'll produce a patch to that effect. I believe
this should go into 9.2 given that otherwise PL/Python will basically
crash any database using the CP12xx encoding.

Cheers,
Jan

Jan Urbański

wulczer@wulczer.org

over 13 years ago

In reply to: Jan Urbański (#5)

1 attachment(s)

Re: plpython issue with Win64 (PG 9.2)

On 03/07/12 17:45, Jan Urbański wrote:

On 29/06/12 00:36, Jan Urbański wrote:

On 27/06/12 13:57, Jan Urbański wrote:

On 27/06/12 11:51, Asif Naeem wrote:

Hi,

On Windows 7 64bit, plpython is causing server crash with the following
test case i.e.

So: I'd add code to translate WINxxx into CPxxx when choosing the Python
to use, change PLy_elog to elog in PLyUnicode_Bytes and leave the
SQL_ASCII case alone, as there were no complaints and people using
SQL_ASCII are asking for it anyway.

Since no one commented, I'll produce a patch to that effect. I believe
this should go into 9.2 given that otherwise PL/Python will basically
crash any database using the CP12xx encoding.

Patch attached. Asif, could you try a few things on a CP1252 database?

First verify if your original test case now works and then try this:

create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('utf-8')
$$ language plpython3u;

select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');

Thanks,
Jan

Attachments:

plpython-windows-encodings.patchtext/x-diff; name=plpython-windows-encodings.patchDownload

diff --git a/src/pl/plpython/plpy_util.c b/src/pl/plpython/plpy_util.c
new file mode 100644
index 9a4901e..5fafbd1
*** a/src/pl/plpython/plpy_util.c
--- b/src/pl/plpython/plpy_util.c
*************** PLyUnicode_Bytes(PyObject *unicode)
*** 66,80 ****
  
  	/*
  	 * Python understands almost all PostgreSQL encoding names, but it doesn't
! 	 * know SQL_ASCII.
  	 */
! 	if (GetDatabaseEncoding() == PG_SQL_ASCII)
! 		serverenc = "ascii";
! 	else
! 		serverenc = GetDatabaseEncodingName();
  	rv = PyUnicode_AsEncodedString(unicode, serverenc, "strict");
! 	if (rv == NULL)
! 		PLy_elog(ERROR, "could not convert Python Unicode object to PostgreSQL server encoding");
  	return rv;
  }
  
--- 66,125 ----
  
  	/*
  	 * Python understands almost all PostgreSQL encoding names, but it doesn't
! 	 * know SQL_ASCII and calls the Windows encodings differently.
  	 */
! 	switch (GetDatabaseEncoding())
! 	{
! 		case PG_SQL_ASCII:
! 			serverenc = "ascii";
! 			break;
! 		case PG_WIN1250:
! 			serverenc = "cp1250";
! 			break;
! 		case PG_WIN1251:
! 			serverenc = "cp1251";
! 			break;
! 		case PG_WIN1252:
! 			serverenc = "cp1252";
! 			break;
! 		case PG_WIN1253:
! 			serverenc = "cp1253";
! 			break;
! 		case PG_WIN1254:
! 			serverenc = "cp1254";
! 			break;
! 		case PG_WIN1255:
! 			serverenc = "cp1255";
! 			break;
! 		case PG_WIN1256:
! 			serverenc = "cp1256";
! 			break;
! 		case PG_WIN1257:
! 			serverenc = "cp1257";
! 			break;
! 		case PG_WIN1258:
! 			serverenc = "cp1258";
! 			break;
! 		case PG_WIN866:
! 			serverenc = "cp866";
! 			break;
! 		case PG_WIN874:
! 			serverenc = "cp874";
! 			break;
! 		default:
! 			serverenc = GetDatabaseEncodingName();
! 			break;
! 	}
! 
  	rv = PyUnicode_AsEncodedString(unicode, serverenc, "strict");
! 	if (rv == NULL) {
! 		/*
! 		 * Use a plan elog instead of PLy_elog here to avoid getting in
! 		 * recursion trouble when the traceback formatting functions try doing
! 		 * unicode to bytes conversion.
! 		 */
! 		elog(ERROR, "could not convert Python Unicode object to PostgreSQL server encoding");
! 	}
  	return rv;
  }

Asif Naeem

asif.naeem@enterprisedb.com

over 13 years ago

In reply to: Jan Urbański (#6)

Re: plpython issue with Win64 (PG 9.2)

Patch attached. Asif, could you try a few things on a CP1252 database?

First verify if your original test case now works and then try this:

I have test the patch on Win64. postgres server is working fine now for
WIN1252. Thanks.

create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('**utf-8')
$$ language plpython3u;

select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');

create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
enctest | encode
----------+--------------------
tróspido | 7472c3b3737069646f
(1 row)

Please do let me know If you have any other query. Thanks.

Best Regards,
Muhammad Asif Naeem

Jan Urbański

wulczer@wulczer.org

over 13 years ago

In reply to: Asif Naeem (#7)

Re: plpython issue with Win64 (PG 9.2)

On 04/07/12 13:58, Asif Naeem wrote:

Patch attached. Asif, could you try a few things on a CP1252 database?

First verify if your original test case now works and then try this:

I have test the patch on Win64. postgres server is working fine now for
WIN1252. Thanks.

create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('**utf-8')
$$ language plpython3u;

select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');

create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
enctest | encode
----------+--------------------
tróspido | 7472c3b3737069646f
(1 row)

Please do let me know If you have any other query. Thanks.

Great, this looks correct.

Can we apply this to 9.2?

Cheers,
Jan

Heikki Linnakangas

heikki.linnakangas@enterprisedb.com

over 13 years ago

In reply to: Jan Urbański (#8)

Re: plpython issue with Win64 (PG 9.2)

On 04.07.2012 15:11, Jan Urbański wrote:

On 04/07/12 13:58, Asif Naeem wrote:

I have test the patch on Win64. postgres server is working fine now for
WIN1252. Thanks.

create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('**utf-8')
$$ language plpython3u;

select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');

create function enctest() returns text as $$
return b'tr\xc3\xb3spido'.decode('utf-8')
$$ language plpython3u;
select enctest(), encode(convert_to(enctest(), 'utf-8'), 'hex');
enctest | encode
----------+--------------------
tróspido | 7472c3b3737069646f
(1 row)

Please do let me know If you have any other query. Thanks.

Great, this looks correct.

Can we apply this to 9.2?

Committed. This bug was present in versions >= 9.0, so backpatched.

I used ereport() rather than elog() in the error message. Correct me if
that was wrong, but the point was to avoid PLy_elog(), because that
might cause recursion, and ereport() should be ok. I believe the message
should be translated, as it's quite possible to get that error, at least
if you use SQL_ASCII, so ereport() is more approriate than elog().

Thanks!

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

#10

Jan Urbański

wulczer@wulczer.org

over 13 years ago

In reply to: Heikki Linnakangas (#9)

Re: plpython issue with Win64 (PG 9.2)

On 05/07/12 21:37, Heikki Linnakangas wrote:

Committed. This bug was present in versions >= 9.0, so backpatched.

Thanks!

I used ereport() rather than elog() in the error message. Correct me if
that was wrong, but the point was to avoid PLy_elog(), because that
might cause recursion, and ereport() should be ok. I believe the message
should be translated, as it's quite possible to get that error, at least
if you use SQL_ASCII, so ereport() is more approriate than elog().

Yes, you're absolutely right.

Cheers,
Jan