Documentation cleanups and add timeouts to Pool.start.

src/gevent/pool.py

@@ -464,10 +464,14 @@ class Group(GroupMappingMixin):
 
     def add(self, greenlet):
         """
-        Begin tracking the greenlet.
+        Begin tracking the *greenlet*.
 
         If this group is :meth:`full`, then this method may block
         until it is possible to track the greenlet.
+
+        Typically the *greenlet* should **not** be started when
+        it is added because if this object blocks in this method,
+        then the *greenlet* may run to completion before it is tracked.
         """
         try:
             rawlink = greenlet.rawlink
@@ -498,8 +502,8 @@ class Group(GroupMappingMixin):
 
     def start(self, greenlet):
         """
-        Start the un-started *greenlet* and add it to the collection of greenlets
-        this group is monitoring.
+        Add the **unstarted** *greenlet* to the collection of greenlets
+        this group is monitoring, nd then start it.
         """
         self.add(greenlet)
         greenlet.start()
@@ -722,24 +726,46 @@ class Pool(Group):
             return 1
         return max(0, self.size - len(self))
 
-    def add(self, greenlet, blocking=True, timeout=None):
+    def start(self, greenlet, *args, **kwargs): # pylint:disable=arguments-differ
+        """
+        start(greenlet, blocking=True, timeout=None) -> None
+
+        Add the **unstarted** *greenlet* to the collection of greenlets
+        this group is monitoring and then start it.
+
+        Parameters are as for :meth:`add`.
         """
-        Begin tracking the given greenlet, possibly blocking until space is
-        available.
+        self.add(greenlet, *args, **kwargs)
+        greenlet.start()
+
+    def add(self, greenlet, blocking=True, timeout=None): # pylint:disable=arguments-differ
+        """
+        Begin tracking the given **unstarted** greenlet, possibly blocking
+        until space is available.
+
+        Usually you should call :meth:`start` to track and start the greenlet
+        instead of using this lower-level method.
 
-        :keyword bool blocking: If True (the default), this function will block
-        until the pool has space or a timeout occurs. If False, this function
-        will immediately raise a Timeout if the pool is currently full.
-        :keyword float timeout: The maximum number of seconds this method will
-        block, if ``blocking`` is True. (Ignored if ``blocking`` is False.)
+        :keyword bool blocking: If True (the default), this function
+            will block until the pool has space or a timeout occurs.  If
+            False, this function will immediately raise a Timeout if the
+            pool is currently full.
+        :keyword float timeout: The maximum number of seconds this
+            method will block, if ``blocking`` is True.  (Ignored if
+            ``blocking`` is False.)
+        :raises PoolFull: if either ``blocking`` is False and the pool
+            was full, or if ``blocking`` is True and ``timeout`` was
+            exceeded.
 
-        Raises `PoolFull` if either ``blocking`` is False and the pool was full,
-        or if ``blocking`` is True and ``timeout`` was exceeded.
+        ..  caution:: If the *greenlet* has already been started and
+            *blocking* is true, then the greenlet may run to completion
+            while the current greenlet blocks waiting to track it. This would
+            enable higher concurrency than desired.
 
-        .. seealso:: :meth:`Group.add`
+        ..  seealso:: :meth:`Group.add`
 
-        .. versionchanged:: 1.3.0
-            Added the ``blocking`` and ```timeout`` parameters.
+        ..  versionchanged:: 1.3.0 Added the ``blocking`` and
+            ``timeout`` parameters.
         """
         if not self._semaphore.acquire(blocking=blocking, timeout=timeout):
             # We failed to acquire the semaphore.

src/greentest/test__pool.py

@@ -205,6 +205,7 @@ class PoolBasicTests(greentest.TestCase):
         finally:
             first.kill()
 
+    @greentest.ignores_leakcheck
     def test_add_method_non_blocking(self):
         p = self.klass(size=1)
         first = gevent.spawn(gevent.sleep, 1000)
@@ -219,6 +220,7 @@ class PoolBasicTests(greentest.TestCase):
         finally:
             first.kill()
 
+    @greentest.ignores_leakcheck
     def test_add_method_timeout(self):
         p = self.klass(size=1)
         first = gevent.spawn(gevent.sleep, 1000)
@@ -233,6 +235,21 @@ class PoolBasicTests(greentest.TestCase):
         finally:
             first.kill()
 
+    @greentest.ignores_leakcheck
+    def test_start_method_timeout(self):
+        p = self.klass(size=1)
+        first = gevent.spawn(gevent.sleep, 1000)
+        try:
+            second = gevent.Greenlet(gevent.sleep, 1000)
+            try:
+                p.add(first)
+                with self.assertRaises(pool.PoolFull):
+                    p.start(second, timeout=0.100)
+            finally:
+                second.kill()
+        finally:
+            first.kill()
+
     def test_apply(self):
         p = self.klass()
         result = p.apply(lambda a: ('foo', a), (1, ))