[Piglit] [Patch v2 1/3] framework.log: Replace synchronized_self decorator

[Dylan Baker]

On Friday, April 25, 2014 16:40:36 Ilia Mirkin wrote:
> On Fri, Apr 25, 2014 at 4:38 PM, Dylan Baker 
<baker.dylan.c at gmail.com> wrote:
> > This replaces the synchronized_self decorator from piglit's threads
> > module with a context manager (with foo: <stuff>). This is more 
standard
> > python, and means having a little less hand rolled code around.
> > 
> > v2: - fix mixed usage of __lock and _lock
> 
> There's probably an obvious reason for this, but I'm missing it -- why
> are you using _lock when all the other vars are hidden with __?

I've been bitten too many times now using __, I'm trying not to use that 
mechanism unless I can come up with a good reason to.

Unless I'm completely misunderstanding python style, a leading _ is the 
equivalent what ruby calls a protected member: That class and all of it's 
children have access to that member. In python it's only suggested that 
method not be called outside of the class. and leading __ is the equivalent 
of a 'private' method, only that class, and not it's children, can access that 
member.

Which means I did the wrong thing when I wrote Log. Unless I completely 
misunderstood?

> 
> > Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> > ---
> > 
> >  framework/log.py | 31 ++++++++++++++++---------------
> >  1 file changed, 16 insertions(+), 15 deletions(-)
> > 
> > diff --git a/framework/log.py b/framework/log.py
> > index d045847..77cc02a 100644
> > --- a/framework/log.py
> > +++ b/framework/log.py
> > @@ -22,7 +22,7 @@
> > 
> >  import sys
> >  import collections
> > 
> > -from .threads import synchronized_self
> > +from threading import RLock
> > 
> >  class Log(object):
> > @@ -38,6 +38,7 @@ class Log(object):
> >          self.__running = []
> >          self.__generator = (x for x in xrange(self.__total))
> >          self.__pad = len(str(self.__total))
> > 
> > +        self._lock = RLock()
> > 
> >          self.__summary_keys = set(['pass', 'fail', 'warn', 'crash',
> >          'skip',
> >          
> >                                     'dmesg-warn', 'dmesg-fail',
> >                                     'dry-run'])
> >          
> >          self.__summary = collections.defaultdict(lambda: 0)
> > 
> > @@ -89,7 +90,6 @@ class Log(object):
> >                                                     'name': name,
> >                                                     'result': result}))
> > 
> > -    @synchronized_self
> > 
> >      def post_log(self, value, result):
> >          """ Used to mark a test as complete in the log
> > 
> > @@ -98,17 +98,17 @@ class Log(object):
> >          result -- the result of the completed test
> >          
> >          """
> > 
> > -        # Mark running complete
> > -        assert value in self.__running
> > -        self.__running.remove(value)
> > +        with self._lock:
> > +            # Mark running complete
> > +            assert value in self.__running
> > +            self.__running.remove(value)
> > 
> > -        # increment the number of completed tests
> > -        self.__complete += 1
> > +            # increment the number of completed tests
> > +            self.__complete += 1
> > 
> > -        assert result in self.__summary_keys
> > -        self.__summary[result] += 1
> > +            assert result in self.__summary_keys
> > +            self.__summary[result] += 1
> > 
> > -    @synchronized_self
> > 
> >      def log(self, name, result):
> >          """ Print to the screen
> > 
> > @@ -116,10 +116,10 @@ class Log(object):
> >          over it.
> >          
> >          """
> > 
> > -        assert result in self.__summary_keys
> > -        self.__print(name, result)
> > +        with self._lock:
> > +            assert result in self.__summary_keys
> > +            self.__print(name, result)
> > 
> > -    @synchronized_self
> > 
> >      def pre_log(self, running=None):
> >          """ Hook to run before log()
> > 
> > @@ -131,8 +131,9 @@ class Log(object):
> >                     nothing will be printed. Default: None
> >          
> >          """
> > 
> > -        if running:
> > -            self.__print(running, 'running')
> > +        with self._lock:
> > +            if running:
> > +                self.__print(running, 'running')
> > 
> >          x = self.__generator.next()
> >          self.__running.append(x)
> > 
> > --
> > 2.0.0.rc0
> > 
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140425/8f06b6b0/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140425/8f06b6b0/attachment-0001.sig>