Race condition in urlstack

[skuzzymiglet]

Expected behavior

Goroutine-safety when calling URLStack

Actual behavior

1. A pointer to an urlstack is passed to Top
2. It's checked for length, evaluates as non-zero, so the function carries on
3. Before it reaches return, the length becomes zero
4. panic!

Steps to reproduce the behavior

Run any gofeed operation concurrently

I think this package needs more testing in concurrent environments. Tons of race conditions are to be found

[skuzzymiglet]

Reproduced in my fork: https://github.com/skuzzymiglet/gofeed/runs/1310765513?check_suite_focus=true#step:6:21 Code: https://github.com/skuzzymiglet/gofeed/blob/concurrent-tests/parser_test.go#L62

[mmcdole]

@skuzzymiglet thanks for the issue.

Definitely looks like some non-thread safe state has been introduced. I'll take a look.

[skuzzymiglet]

In my (unstable) fork, I'm thinking of folding URLStack into XMLBase, since it's the only user.

Also, what is XMLBase? I have no clue.

[cristoper]

Also, what is XMLBase? I have no clue.

XMLBase implements resolving relative URLs according to the xml:base XML attribute. It has to keep track of the most recently defined base URL at the current scope. One way to remove the race condition when using Parser concurrently would be to move management of the XMLBase state from the Parser to each Feed (see #201 )