Fixes for Go 1.4 #79
Fixes for Go 1.4 #79
Conversation
Previously SetResolver was taking a pointer to an argument, which is stack allocated.
`serialize` was giving a pointer into it's stack to `xmlNodeWriteCallback`. If the runtime moves our stack, that pointer will be invalid. Instead we use the pointer as an integer to index a map, as recommended here golang/go#8310
Previous commit forgot the lock. Maps are not safe for concurrent access.
|
To anyone considering gokogiri with Go 1.4+, you really need this fix. Things will probably work fine at low traffic, then break up when that increases. Please use this version with the fixes: https://github.com/ThomsonReutersEikon/gokogiri |
|
I have to agree with @grahamking -- just yesterday I updated ratago to use their fork until this PR is merged. |
|
First, thanks for the PR Graham. We have considered a slightly different approach for one of the problems you have addressed. Instead of having a callback into go and managing the lock, we instead accumulated the data in C and only perform the callback once. I hope to get this resolved with the rest of your PR and get this merged in in the next two weeks. |
|
I tried |
|
I'm afraid I don't know of one. The Thomson Reuters project that was using gokogiri switched to etree a while back, all their XML parsing is pure Go now. The Go<->C rules are finally explicit in Go 1.6, so I'm not surprised there is work needed. |
|
@pebbe I'll have a look this weekend as I need to update ratago for 1.6 and will submit a PR to both forks. |
Two separate fixes for Go 1.4. It isn't safe to pass a pointer into a function's stack since 1.3, but this seems to blow up much more often in 1.4, maybe because the stack is smaller, so is more likely to be moved.
The xpath commit fixes a test failure in
TestEvalVariableExpr. The serialize commit has it's own test.Details of the general problem here: golang/go#8310
Replaces #78