Document threading contract for Session class

[gward]

Right now, it's quite difficult to figure out if the Session class is threadsafe or not. The docs don't say, apart from a "thread-safe" bullet on the home page. Google led me to http://stackoverflow.com/questions/18188044/is-the-session-object-from-pythons-requests-library-thread-safe, whose first answer boils down to "be very careful".

Inspired by that SO author, I've been auditing the code myself, and have come to the conclusion that Session is probably not threadsafe. (The use of self.redirect_cache set off red flags for me.) Reading through other requests bug reports, I see maintainers recommending one Session per thread, which implies that it's not threadsafe.

The documentation should clarify this and recommend how to use Session in multithreaded programs. Possible text:

Session is not threadsafe. If you are using requests with an explicit Session
object in a multithreaded program, you should create one Session per thread.

If that's accurate, let me know and I'll submit a PR.

Also, I think the "thread-safe" bullet should be removed from the home page, or maybe replaced by "thread-safe in certain circumstances".

[sigmavirus24]

Why do you think the redirect cache is not threadsafe?

[Lukasa]

I'd go further: I think the redirect cache is a bad idea. I still think that.

I think re-adding thread-safety would be a good idea. We can get away with it, I think, by locking around the CookieJar (which should be the only other thing that is a risk here). That's a cheap-and-easy way to get something approximating thread safety, even if it doesn't necessarily get great performance.

[aaugustin]

It would be pretty cool if requests.Session was guarateed to be safe.

This would make that sort of code safe by default.

This sounds better than pestering every maintainer of every API client based on requests to make their Session objects thread-local :-)

I don't expect a large overhead from locking around writes to the cookie jar. Requests is supposed to spend most of its time waiting for I/O.

[Lukasa]

The cookie jar, sadly, is not the only concern: the entire Session class needs auditing for thread safety. It's not a bad idea, it's just a matter of finding someone with the time to spare to do it. =)

[kennethreitz]

Sounds like someone should write a tool that makes this really easy :P

[mmerickel]

What is the motivation for the "thread-safety" bullet point on the main page? I can't find the word "thread" almost anywhere in the documentation and definitely nothing about thread safety. This issue alongside several others indicate that the session is not actually thread-safe. The bullet point really should be removed as it seems unfounded right now.

[reversefold]

I've opened an issue with urllib3 as the main underlying issue appears to be with PoolManager and HTTPConnectionPool, both of which are part of urllib3 and not requests. https://github.com/shazow/urllib3/issues/1252

[madsmtm]

From what I can tell, @reversefold's PR culminated with https://github.com/urllib3/urllib3/pull/1263, but looking at the work he did, it didn't sound impossible. So assuming we can fix that, what is the current state of thread safety in requests? I know that the redirect cache was removed in https://github.com/kennethreitz/requests/pull/4100, but @Lukasa mentioned that the whole Session class needs to be audited, is this something I could help with?

As a general question, is this downprioritized in favor of async stuff like https://github.com/kennethreitz/requests/issues/1390? (And if so, should I put my full attention there as well? 😉)

(For future reference, the note about thread safety was removed in 66293b830a8b9507a458d26e305664bf4ea03d55)

[Dr-Emann]

Any official word on this? There's been a lot of foundational work in urllib3 to be thread safe, is the official recommendation still "one session per thread"?

[dimaqq]

I too would like an official statement on this.

[david-gang]

I've opened an issue with urllib3 as the main underlying issue appears to be with PoolManager and HTTPConnectionPool, both of which are part of urllib3 and not requests. urllib3/urllib3#1252

looks like urllib is thread safe now: https://github.com/urllib3/urllib3/pull/2661

[sethmlarson]

Closing this until a separate threading issue is shown in Requests, please open a new issue if that's the case.