Make XMLBase thread-safe #201
Conversation
It was not safe to concurrently parse feeds with a single Parser instance because each Parser shares an XMLBase urlStack (to resolve URLs relative to the closes xml:base) With this commit, instead of sharing an XMLBase, each feed maintains its own urlStack and can be parsed concurrently. Should fix mmcdole#160 and hopefully mmcdole#159 To test: go test -race
|
Thank you @cristoper so much. I was literally gearing up to do something similar: pull XMLBase from an instance property of the parser struct and make it create one per parse execution. |
|
I've wondered, if it makes sense to pull XMLBase into XPP, and make it a part of the pull parser itself, or if it is more appropriate as it as, a part of Gofeed. |
|
Ah, I think keeping track of the xml:base url stack in XPP makes a lot of sense since it is keeping document state as it parses anyway. But I don't think XPP should actually rewrite any URLs in elements or attributes based on xml:base; that should be up to applications. I've got a version of XPP and Gofeed working where the url stack is kept by XPP and Gofeed uses it to resolve URLs as it parses. I'll submit a couple of PRs so you can take a look. |
|
Closing this since we decided to go with the solution in #202 instead |
Hey @mmcdole -
I'm the original author of the XMLBase code (#101). I've recently returned to using gofeed in a new project and noticed that my xmlbase implementation is the source of some concurrency issues (#159 , #160 ). One way to allow Parser to be used concurrently would be to have each Feed manager its own urlStack instead of having a single shared urlStack managed by Parser. That's what this PR does. (The first commit is a test which uses a Parser from several go routines; when run with
go test -raceit should fail without the second commit and pass with the commit applied.)