Page MenuHomePhabricator

fetch hangs if a httplib2.RedirectLimit occurred
Closed, ResolvedPublic

Description

When a httplib2.RedirectLimit (maybe also with others) occurred on pywikibot.comms.http.fetch it hangs:

>>> import pywikibot.comms.http
>>> pywikibot.comms.http.fetch('http://www.tvtropes.org/api.php')
Traceback (most recent call last):
  File "/home/xzise/.pyenv/versions/3.4.3/lib/python3.4/threading.py", line 920, in _bootstrap_inner
    self.run()
  File "/home/xzise/Programms/pywikibot/core/pywikibot/comms/threadedhttp.py", line 549, in run
    item.data = self.http.request(*item.args, **item.kwargs)
  File "/home/xzise/Programms/pywikibot/core/pywikibot/comms/threadedhttp.py", line 260, in request
    uri, method, body, headers, response, content, max_redirects)
  File "/home/xzise/Programms/pywikibot/core/pywikibot/comms/threadedhttp.py", line 307, in _follow_redirect
    max_redirects=max_redirects - 1)
  File "/home/xzise/Programms/pywikibot/core/pywikibot/comms/threadedhttp.py", line 260, in request
    uri, method, body, headers, response, content, max_redirects)
  File "/home/xzise/Programms/pywikibot/core/pywikibot/comms/threadedhttp.py", line 311, in _follow_redirect
    response, content)
httplib2.RedirectLimit: Redirected more times than redirection_limit allows.

^CTraceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/xzise/Programms/pywikibot/core/pywikibot/comms/http.py", line 364, in fetch
    error_handling_callback(request)
  File "/home/xzise/Programms/pywikibot/core/pywikibot/comms/http.py", line 274, in error_handling_callback
    if isinstance(request.data, SSLHandshakeError):
  File "/home/xzise/Programms/pywikibot/core/pywikibot/comms/threadedhttp.py", line 392, in data
    self._join()
  File "/home/xzise/Programms/pywikibot/core/pywikibot/comms/threadedhttp.py", line 386, in _join
    self.lock.acquire(True)
  File "/home/xzise/Programms/pywikibot/core/pywikibot/comms/threadedhttp.py", line 323, in acquire
    return super(S, self).acquire(blocking)
  File "/home/xzise/.pyenv/versions/3.4.3/lib/python3.4/threading.py", line 421, in acquire
    self._cond.wait(timeout)
  File "/home/xzise/.pyenv/versions/3.4.3/lib/python3.4/threading.py", line 290, in wait
    waiter.acquire()
KeyboardInterrupt

From what I could tell it's not releasing the semaphore correctly (although when looking at the code it should and it does actually call that) and when it then tries to check for errors it tries to acquire it and waits then on it.

try:
    # inside of that it crashes
    item.data = self.http.request(*item.args, **item.kwargs)
finally:
    if item.lock:
        # this line is called
        item.lock.release()

I tried to determine where it acquires the lock but only find the one place in fetch and then one when it tries to get the data in error_handling_callback.

(Note: The line numbers are bit off as I try to debug that one)

Event Timeline

XZise raised the priority of this task from to Needs Triage.
XZise updated the task description. (Show Details)
XZise added a project: Pywikibot.
XZise added subscribers: XZise, Omegat.
Restricted Application added subscribers: Aklapper, Unknown Object (MLST). · View Herald TranscriptApr 3 2015, 10:32 AM

Ah I think it's not setting threadedhttp.HttpRequest._data when that exception occurs and thus it tries to acquire the semaphore. Another problem is that fetch does acquire the semaphore but doesn't release it. I'm not sure if that is intentional (that way after the data has been fetched the lock is blocked so when a request happens it can't acquire the lock).

Change 201696 had a related patch set uploaded (by XZise):
[FIX] Return exception instead of raising it

https://gerrit.wikimedia.org/r/201696

There is another bug about this problem, discussing a solution which catches raised exceptions.

Could you specify? Is there a bug already existing on Phabricator, I couldn't find really one.

ugh. im having difficulty finding it. :/

Not sure. I do remember something about it, but I can't find a bug either -- maybe it's in a gerrit comment instead?

Change 201696 merged by jenkins-bot:
[FIX] Return exception instead of raising it

https://gerrit.wikimedia.org/r/201696

jayvdb claimed this task.

I looked in a few gerrit's and couldnt find it. The revised patch almost certainly addresses the core of the problem that was previously discussed.

jayvdb set Security to None.