Commit f2adbb1065eb introduced abort in _csv module in debug builds in Python 3.3, when using APSW shell.

(You need to have SQLite >=3.7.14 installed.)

$ cd /tmp
$ wget http://apsw.googlecode.com/files/apsw-3.7.14-r2.zip
$ unzip apsw-3.7.14-r2.zip
$ cd apsw-3.7.14-r2
$ python3.3dm setup.py build
...
$ PYTHONPATH="$(ls -d build/lib*)" python3.3dm tools/shell.py
SQLite version 3.7.14.1 (APSW 3.7.14-r2)
Enter ".help" for instructions
Enter SQL statements terminated with a ";"
sqlite> .output output_file
sqlite> .mode csv
sqlite> select "a";
python3.3dm: /tmp/cpython/Modules/_csv.c:1111: join_append: Assertion `(((PyASCIIObject*)field)->state.ready)' failed.
Aborted
$ 

In bash shell:
$ PYTHONPATH="$(ls -d build/lib*)" python3.3dm tools/shell.py <<< $'.output output_file\n.mode csv\nselect "a";'
python3.3dm: /tmp/cpython/Modules/_csv.c:1111: join_append: Assertion `(((PyASCIIObject*)field)->state.ready)' failed.
Aborted
$

Well, can you try to enable core dumps and get a complete stack trace?

I'm the APSW author.  You do not need SQLite installed - APSW's setup can fetch the current SQLite and use it privately not affecting the rest of the system.

An easier way of testing is:

  python3 setup.py fetch --sqlite --version 3.7.14 build_ext --inplace --force

Create test.sql with these contents

  .output output_file
  .mode csv
  select 3, '4';

Then run:

  env PYTHONPATH=. python3 tools/shell.py
  .read test.sql

The actual code where the problem happens does the following:

- Create a StringIO
- Initialize csv.writer using the StringIO
- Write one row with two values (this is when the crash happens in Python 3.3 on the first write)
- Copy the current contents of the StringIO to the actual output file changing encoding as needed
- Truncate StringIO and seek back to offset zero ready for the next line

The relevant code is in tools/shell.py in the output_csv function.  Writing just that sequence of code above doesn't result in the assertion.  valgrind doesn't show any problems either (using pydebug, without pymalloc and all the freelists set to zero).

A stack trace is here:  https://code.google.com/p/apsw/issues/detail?id=132#c4

If not using a debug/nopymalloc build then you get a crash happening later.

In general APSW does not look ready for the Python 3.3 Unicode
implementation. Example: src/statementcache.c:217

    Py_UNICODE *out;
    PyObject *res=PyUnicode_FromUnicode(NULL, size);


Python 3.3 docs:

"If the buffer is NULL, PyUnicode_READY() must be called once the string content has been filled before using any of the access macros such as PyUnicode_KIND()."


The abort() in the traceback occurs precisely because PyUnicode_READY()
hasn't been called, so I'm closing this as invalid.

How do you get values to write? Most likely, the problem has nothing to do with csv module, but written values are broken. Possible error in sqlite module, or in any conversion function, or in you APSW C-code. Can you dump this values before sending to writerow?

Oh, I see the issue has already been solved.

My understanding was that code that calls the public APIs should continue to work.  Is this a new requirement in 3.3, or is it that it has always been a requirement but code could get away without the ready before?  

Either way we need to add an note to the Porting section of What's New.

Apparently http://docs.python.org/py3k/whatsnew/3.3.html#pep-393-flexible-string-representation says:
"On the C API side, PEP 393 is fully backward compatible."

I'm going to reopen this at least until we have more information.

R. David Murray <report@bugs.python.org> wrote:
> My understanding was that code that calls the public APIs should continue to
> work. Is this a new requirement in 3.3, or is it that it has always been a
> requirement but code could get away without the ready before?  

It's a new requirement.

> Either way we need to add an note to the Porting section of What's New.

Good point. The main hint I find is here:

http://docs.python.org/py3k/whatsnew/3.3.html#deprecated-functions-and-types-of-the-c-api

But you have to click through to:

http://docs.python.org/py3k/c-api/unicode.html#PyUnicode_FromUnicode

PEP 393 on the other hand has this paragraph:

"Extension modules using the legacy API may inadvertently call PyUnicode_READY,
 by calling some API that requires that the object is ready, and then continue
 accessing the (now invalid) Py_UNICODE pointer. Such code will break with this
 PEP. The code was already flawed in 3.2, as there is was no explicit guarantee
 that the PyUnicode_AS_UNICODE result would stay valid after an API call (due
 to the possibility of string resizing). Modules that face this issue need to
 re-fetch the Py_UNICODE pointer after API calls; doing so will continue to
 work correctly in earlier Python versions."

David is right, the existing (APSW) code should continue to work.
I think we need to add a couple of PyUnicode_READY() to some
functions in Modules/_csv.c.

"I think we need to add a couple of PyUnicode_READY() to some functions in Modules/_csv.c."

Oh yes, the _csv module was in my TODO list. I forgot to fix it :-/

"In general APSW does not look ready for the Python 3.3 Unicode implementation."

I bet that most Python modules implemented in C use the "legacy" (old) Unicode API (Py_UNICODE*). Python 3.3 must call PyUnicode_READY() everywhere (where the new API is used to get attribute of a string, ex: PyUnicode_GET_LENGTH).

(APSW author here).  I haven't ported to the new Python 3.3 Unicode handling yet but it is on my todo list.  The PEPs and doc said the C API would remain backwards compatible.  The APSW code supports Python 2.3 onwards.

SQLite APIs support both UTF-8 and UTF-16.  Py_UNICODE_SIZE is used to select between two code paths - if 2 then the UTF-16 APIs are used and if 4 then a UTF-8 conversion is made and those APIs used.  Python 3.3 makes Py_UNICODE_SIZE meaningless and will be quite a bit of work to integrate as a third branch.

It should be noted that at no point is there any dependence of the underlying bytes/pointer living a long time.  They are only used at the point which data needs to be transferred between Python types and SQLite APIs (both ways) and not held outside of that period.

Here's a patch.


Victor, I guess I have a feature request for fusil: It would be nice
if fusil also generated legacy strings (or does it already do so?).

> The PEPs and doc said the C API would remain backwards compatible.

Roger: Yes, you don't need to change anything. I was misled by the
PyUnicode_AsUnicode() docs, thinking that the responsibility to call
PyUnicode_READY() also applies if the generated Unicode Object is
passed to the C-API.

Roughly how long will it be before Python 3.3.1 comes out?  This issue means my users will get garbage or crashes, so I'll need to work around it if it will be quite a while till 3.3.1.

3.3.1 will not be too long: maybe 4 weeks from now.

LGTM

LGTM. But I left some minor comments in Rietveld.

Hmm.  Is it a bugtracker bug?  I don't know how I added Larry to the nosy list and now I can not remove him.

I don't know.  I was able to remove him.  (I have javascript turned off, don't know if that makes any difference).

Probably Versions="Python 3.4" + Priority="release blocker" results in addition of 3.4 Release Manager (Larry Hastings) to nosy list.

Roundup knows I'm the release manager for 3.4?  It's well-informed! ;-)

New changeset 0b5f00ccf907 by Stefan Krah in branch '3.3':
Issue #16145: Support legacy strings in the _csv module.
http://hg.python.org/cpython/rev/0b5f00ccf907

Andrew and Serhiy, thanks for the reviews. Should be fixed now.

So is 3.3.1 with the fix ever going to be released?  Georg did predict mid-November and we are 4 months after that.

It won't be too much longer.