Skip to content

Conversation

Komnomnomnom
Copy link
Contributor

Fixes for ujson #1387

First commit fixes mingw build issues. Builds ok and tests pass on my mingw and VS 2008, also tested on OSX. Needs testing on enthought mingw though.

More commits to come.

@wesm
Copy link
Member

wesm commented Jun 5, 2012

Merged in. Build passes on mingw32 gcc 4.5.2 in EPD 7.1 but still segfaults; will check a newer mingw32

@wesm wesm closed this Jun 5, 2012
@Komnomnomnom
Copy link
Contributor Author

I was going to push more commits for ujson onto this branch / pull-request as I fix the various issues. Saves creating multiple branches / pull requests?

@wesm wesm reopened this Jun 5, 2012
@wesm
Copy link
Member

wesm commented Jun 5, 2012

Got it, got a bit carried away. I'll leave the PR open, sorry about the inconvenience

@Komnomnomnom
Copy link
Contributor Author

Apologies for this getting a bit messy. I needed to cherry pick some commits from master to make the build succeed.

@Komnomnomnom
Copy link
Contributor Author

Latest commit prevents segmentation faults if ujson runs out of memory (or malloc fails for some other reason).

Still need to look into mingw segmentation faults but I can confirm reproducing them for EPD 7.1. My first guess is that it might be related to Python being built with VS 2008 and the extension built with mingw...

@wesm
Copy link
Member

wesm commented Jun 7, 2012

Cool. I'm going to check EPD 7.3 and see if it's better. Still haven't gotten anything definitive from the ujson folks re: licensing-- if we haven't been able to resolve the segfaults within a few days will probably just factor out the extension so the release isn't held up. A bunch of other issues to close between now and then + lots of line test coverage to do

@wesm
Copy link
Member

wesm commented Jun 10, 2012

I merged all these commits. I'll separate out the ujson extension today or tomorrow into a separate installable project (probably called pandasjson or something uninspiring) under pydata on GitHub so this doesn't hold up the release.

@wesm wesm closed this Jun 10, 2012
@Komnomnomnom
Copy link
Contributor Author

Ok np. These last few fixes were motivated by testing on Yaroslav's SPARC box.

Note I've tested with EPD 7.2 & 7.3 on Windows. No joy, still seeing the same problems, and my attempts at getting a Python debug build in windows have so far only ended in frustration....

With my own mingw the tests pass but I've seen a very occasional segmentation fault with an MSVC compilation. Happened twice during about 20 test runs, I haven't been able to reproduce it since but the one time it happened while running nose in verbose the segmentation fault occurred in test_align_fill_method.

I had some relevant valgrind output but my VM's just crashed. Will append it here once I get it back.

Edit: test_align_fill_method is in test_frame.py

@Komnomnomnom
Copy link
Contributor Author

valgrind on Ubuntu 32bit VM

test_align_fill_method (pandas.tests.test_frame.TestDataFrame) ... 
==6029== Invalid read of size 8
==6029==    at 0x78A6D13: __pyx_pw_6pandas_6_algos_71backfill_2d_inplace_float64 (generated.c:17109)
==6029==    by 0x80DA99F: PyEval_EvalFrameEx (ceval.c:4236)
==6029==    by 0x80DCD50: PyEval_EvalCodeEx (ceval.c:3253)
==6029==    by 0x80DB181: PyEval_EvalFrameEx (ceval.c:4117)
==6029==    by 0x80DCD50: PyEval_EvalCodeEx (ceval.c:3253)
==6029==    by 0x80DB181: PyEval_EvalFrameEx (ceval.c:4117)
==6029==    by 0x80DCD50: PyEval_EvalCodeEx (ceval.c:3253)
==6029==    by 0x80DB181: PyEval_EvalFrameEx (ceval.c:4117)
==6029==    by 0x80DCD50: PyEval_EvalCodeEx (ceval.c:3253)
==6029==    by 0x80DB181: PyEval_EvalFrameEx (ceval.c:4117)
==6029==    by 0x80DCD50: PyEval_EvalCodeEx (ceval.c:3253)
==6029==    by 0x80DB181: PyEval_EvalFrameEx (ceval.c:4117)
==6029==  Address 0xb916560 is 8 bytes before a block of size 32 alloc'd
==6029==    at 0x402BE68: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==6029==    by 0x6BA4013: PyArray_NewFromDescr (ctors.c:1086)
==6029==    by 0x6BA43A3: PyArray_NewLikeArray (ctors.c:1214)
==6029==    by 0x6BC0B25: array_copy (convert.c:470)
==6029==    by 0x80DAADB: PyEval_EvalFrameEx (ceval.c:4021)
==6029==    by 0x80DCD50: PyEval_EvalCodeEx (ceval.c:3253)
==6029==    by 0x80DB181: PyEval_EvalFrameEx (ceval.c:4117)
==6029==    by 0x80DCD50: PyEval_EvalCodeEx (ceval.c:3253)
==6029==    by 0x80DB181: PyEval_EvalFrameEx (ceval.c:4117)
==6029==    by 0x80DCD50: PyEval_EvalCodeEx (ceval.c:3253)
==6029==    by 0x80DB181: PyEval_EvalFrameEx (ceval.c:4117)
==6029==    by 0x80DCD50: PyEval_EvalCodeEx (ceval.c:3253)
==6029== 
ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants