Race-Condition about cuttlepool HOT 6 CLOSED

Yup, that’s a race condition. Thanks for finding that bugger. I can wrap get_resource with the lock which should solve the problem. I’ll have this fixed today.

from cuttlepool.

Resolved in PR #37.

from cuttlepool.

from cuttlepool.

Yes, I think you're right. The lock is really meant to be used to prevent race conditions when accessing the queue, not individual resources. _get and _make_resource should return the wrapped resource as opposed to the tracker. If you are making changes, a PR would be greatly appreciated.

from cuttlepool.

(..sorry for answering late - I'd some days "off"...)
The problem with the function "get_resource" as you have it in the version 0.9 is the call to "ping()":

  def get_resource(self, resource_wrapper=None):
    ...
    with self._lock:
        ...
        # Ensure resource is active.
        if not self.ping(rtracker.resource):
           ...

So, the pool it locked until the function ping() returns which may take a long time if - for example - one node of the Resource-Cluster is down. Think of a case where 10 free resources are in the pool, 11 client threads did call "get_resource()" and "ping()" has 30s timeout until it notices that the resource has moved to an other host. The last client call would have to wait for 300s (!) until it gets

a fesh resource....

I suggest the following:

1. Change the function "_get()" to return both, the _ResourceTracker object and the wrapped resource:

   def _get(self, timeout, resource_wrapper_class):
       with self_lock:
           ...
           return rtracker, rtracker.wrap_resource(self, resource_wrapper_class)
   

2. Change the function "_make_resource()" to return both, the_ResourceTracker object and the wrapped resource:

   def _make_resource(self, resource_wrapper_class):
       with self_lock:
           ...
           return rtracker, rtracker.wrap_resource(self, resource_wrapper_class)
   

3. Change the function "get_resource()" so that "_ResourceTracker" and the wrapped resource are stored when calling "_get()" or "_make_resource()". Example:

    try:
        # try the get a resource from the pool. Do not wait; pool is empty at very first call
        rtracker, wrapedResource = self._get(0, resource_wrapper)
    except PoolEmptyError:
        pass
   

4. Remove the statement with self._lock: at line 300 in the function "get_resource()" - this is not needed anymore.

I append a changed version of the file - but sorry - it was not based on the "0.9" version of your's,,,,

cuttlepool.zip

from cuttlepool.

Overall, this looks good. I think if the wrapped resource is being returned by _make_resource and _get, then the rtracker doesn't need to be returned as well. Can you make a pull request with these changes?

from cuttlepool.