[PATCH 2 of 2 V2] hgweb: consume generator inside context manager (issue4756)

[Gregory Szorc]

On Mon, Sep 14, 2015 at 4:05 PM, Pierre-Yves David <
pierre-yves.david at ens-lyon.org> wrote:

>
>
> On 09/12/2015 12:18 PM, Gregory Szorc wrote:
>
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc at gmail.com>
>> # Date 1442085399 25200
>> #      Sat Sep 12 12:16:39 2015 -0700
>> # Node ID 5073c7fb4632855becb31dbd315d70c03c8a2309
>> # Parent  fd8dc94b2bd2c57b0d9669ef9708c655f8a0859c
>> hgweb: consume generator inside context manager (issue4756)
>>
>
> patch 1 seems to have been queue from v1, what is the status of this?
>

I resent patch 1 in the V2 because the patches were related to hgweb thread
safety. As long as both are queued, we should be fine.


>
> If code inside a context manager returns a generator, the context
>> manager exits before the generator is iterated.
>>
>> hgweb was using a context manager to control thread safe access to a
>> localrepository instance. But it was returning a generator, so there was
>> a race condition between a previous request streaming a response to the
>> client and a new request obtaining the released but in use repository.
>> By iterating the generator inside the context manager, we ensure we
>> don't release the repo instance until after the response has finished.
>>
>> With this change, hgweb finally appears to have full localrepository
>> isolation between threads. I can no longer reproduce the 2 exceptions
>> reported in issue4756.
>>
>> test-hgweb-non-interactive.t has been modified to consume the output
>> of calling into a WSGI application. Without this, execution of the WSGI
>> application stalls because of the added yield statement.
>>
>> diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
>> --- a/mercurial/hgweb/hgweb_mod.py
>> +++ b/mercurial/hgweb/hgweb_mod.py
>> @@ -274,9 +274,10 @@ class hgweb(object):
>>           This is typically only called by Mercurial. External consumers
>>           should be using instances of this class as the WSGI application.
>>           """
>>           with self._obtainrepo() as repo:
>> -            return self._runwsgi(req, repo)
>> +            for r in self._runwsgi(req, repo):
>> +                yield r
>>
>
> nice catch, you should probably document it.
>
>
>>       def _runwsgi(self, req, repo):
>>           rctx = requestcontext(self, repo)
>>
>> diff --git a/tests/test-hgweb-non-interactive.t
>> b/tests/test-hgweb-non-interactive.t
>> --- a/tests/test-hgweb-non-interactive.t
>> +++ b/tests/test-hgweb-non-interactive.t
>> @@ -57,9 +57,10 @@ by the WSGI standard and strictly implem
>>     >     'SERVER_PROTOCOL': 'HTTP/1.0'
>>     > }
>>     >
>>     > i = hgweb('.')
>> -  > i(env, startrsp)
>> +  > for c in i(env, startrsp):
>> +  >     pass
>>     > print '---- ERRORS'
>>     > print errors.getvalue()
>>     > print '---- OS.ENVIRON wsgi variables'
>>     > print sorted([x for x in os.environ if x.startswith('wsgi')])
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at selenic.com
>> https://selenic.com/mailman/listinfo/mercurial-devel
>>
>>
> --
> Pierre-Yves David
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150914/f1db3136/attachment.html>