[issue23247] Crash in the reset() method of StreamWriter of CJK codecs

STINNER Victor added the comment: @Aaron Hill: Thanks for your patch! I only kept the minimal test of your patch. If you want to enhance the test suite, you may write new test to test the behaviour of reset(). I prefer to only commit the minimum patch. @Martin: Thanks for your report. The issu

[issue23247] Crash in the reset() method of StreamWriter of CJK codecs

Roundup Robot added the comment: New changeset 35a6fe0e2b27 by Victor Stinner in branch '3.4': Closes #23247: Fix a crash in the StreamWriter.reset() of CJK codecs https://hg.python.org/cpython/rev/35a6fe0e2b27 -- nosy: +python-dev resolution: -> fixed stage: patch review -> resolved st

[issue23247] Crash in the reset() method of StreamWriter of CJK codecs

Aaron Hill added the comment: I've added a test case to exercise reset() -- Added file: http://bugs.python.org/file39934/fix-multibytecodec-segfault-with-test.patch ___ Python tracker _

[issue23247] Crash in the reset() method of StreamWriter of CJK codecs

STINNER Victor added the comment: > Aaron: Your version of the fix immediately returns None, while Victor’s tries > to encode an empty string (if I understand it correctly). Aaron patch is better. I misunderstood the code. reset() always return None, the empty string is unused in my patch. fi

[issue23247] Crash in the reset() method of StreamWriter of CJK codecs

Martin Panter added the comment: Aaron: Your version of the fix immediately returns None, while Victor’s tries to encode an empty string (if I understand it correctly). I imagine this shortcut could be slightly more efficient, but is it always correct? In any case, Aaron’s test looks okay to m

[issue23247] Crash in the reset() method of StreamWriter of CJK codecs

Aaron Hill added the comment: The patch didn't get attached for some reason. It's attached now. -- Added file: http://bugs.python.org/file39933/fix-multibytecodec-segfault.patch ___ Python tracker _

[issue23247] Crash in the reset() method of StreamWriter of CJK codecs

Serhiy Storchaka added the comment: cjk_codecs_reset.patch LGTM. -- assignee: serhiy.storchaka -> haypo stage: -> test needed ___ Python tracker ___

[issue23247] Crash in the reset() method of StreamWriter of CJK codecs

Aaron Hill added the comment: The included patch fixes the issue, and modifies the existing unittest to prevent a future regression. The patch corrects an issue where the 'pending' struct field was NULL, but was used as the input to multibytecodec_encode anyay. -- ___

[issue23247] Crash in the reset() method of StreamWriter of CJK codecs

Martin Panter added the comment: Perhaps this test case proposed in my patch for Issue 13881 could be useful: +def test_writer_reuse(self): +"""StreamWriter should be reusable after reset""" Looks like that is where my “broken_stream_codecs” list from the original post came from.

[issue23247] Crash in the reset() method of StreamWriter of CJK codecs

STINNER Victor added the comment: > It looks like there was no test for this specific bug :-/ Calling reset() > just after creating a StreamReader object. Oops: StreamWriter, not StreamReader. -- ___ Python tracker

[issue23247] Crash in the reset() method of StreamWriter of CJK codecs

STINNER Victor added the comment: > Happens for all the multibyte codecs: (...) All these codecs share the same C implementation: the "CJK" codecs. The bug was introduced in Python 3.4 by my huge changeset d621bdaed7c3: Issue "#17693: CJK encoders now use the new Unicode API (PEP 393)". It lo