[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Guido van Rossum]

Guido van Rossum added the comment:

Correct, if you want to work on it, see 
http://code.google.com/p/tulip/issues/detail?id=114

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Yury Selivanov]

Yury Selivanov added the comment:

This one is still not merged in Tulip, right?

--
nosy: +yselivanov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[STINNER Victor]

Changes by STINNER Victor :


--
resolution: remind -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Roundup Robot]

Roundup Robot added the comment:

New changeset 791b69f9f96d by Victor Stinner in branch '3.3':
Issue #20367: Fix behavior of concurrent.futures.as_completed() for duplicate
http://hg.python.org/cpython/rev/791b69f9f96d

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Glenn Langford]

Glenn Langford added the comment:

@Victor: Thank you, and I appreciate all your advice! I am still learning the 
dev environment but hope to be helpful. :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Guido van Rossum]

Guido van Rossum added the comment:

Sorry, I suppose it needs to be backported to 3.3.

If someone wants to do that, please do (I'm afraid I'd mess up the merge).

--
resolution: fixed -> remind
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[STINNER Victor]

STINNER Victor added the comment:

@Guido: Why not fixing the issue in Python 3.3? You forgot to add Gleen 
Langford to Misc/ACKS!

@Gleen: Congrats for your first commit :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Roundup Robot]

Roundup Robot added the comment:

New changeset 1dac8c954488 by Victor Stinner in branch 'default':
Issue #20367: Add Glenn Langford to Misc/ACKS
http://hg.python.org/cpython/rev/1dac8c954488

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Guido van Rossum]

Changes by Guido van Rossum :


--
resolution:  -> fixed
stage:  -> committed/rejected
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Roundup Robot]

Roundup Robot added the comment:

New changeset 58b0f3e1ddf8 by Guido van Rossum in branch 'default':
Fix issue #20367: concurrent.futures.as_completed() for duplicate arguments.
http://hg.python.org/cpython/rev/58b0f3e1ddf8

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Glenn Langford]

Glenn Langford added the comment:

Ah...ok, here is a patch that includes an update to 
Doc/library/concurrent.futures.rst as well.

--
Added file: http://bugs.python.org/file33728/issue20367.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Guido van Rossum]

Guido van Rossum added the comment:

LGTM. But you also need to update Doc/library/concurrent.futures.rst

I see this as a bugfix so it's not necessary to get this in before the beta 3 
release tonight.

I will work on an asyncio patch and test.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Glenn Langford]

Glenn Langford added the comment:

Thanks for the feedback. The new patch is modified for PEP8 with naming 
consistent with other concurrent tests. assertEqual I think is clearer by 
checking list length, so it is not changed. The docstring is updated.

I suggest asyncio be handled separately.

--
Added file: http://bugs.python.org/file33686/issue20367.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[STINNER Victor]

STINNER Victor added the comment:

Hum, you should also modify the documentation to explicit the
behaviour. Example: "Duplicates futures are only yielded once".

You may add the same sentence in the asyncio.as_completed()
documentation. It looks like asyncio tests doesn't test as_completed()
with duplicated future. You may write a new patch to modify asyncio
doc and tests. It should be very similar.

+completed = [f for f in futures.as_completed( [f1,f1] ) ]

You can just use list(futures.as_completed([f1,f1])). Please no space
around parenthesis (see the PEP 8).

+self.assertEqual( len(completed), 1 )

No space around parenthesis (see the PEP 8):
self.assertEqual(len(completed), 1).

You may check the list value instead: self.assertEqual(completed, [f1])

(Why "f1" name? There is no f2, maybe rename to f?)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Glenn Langford]

Glenn Langford added the comment:

> Could you please try to write a unit test.

Revised patch with unit test for as_completed().

--
Added file: http://bugs.python.org/file33685/issue20367.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[STINNER Victor]

STINNER Victor added the comment:

> Proposed patch for as_completed().

Could you please try to write a unit test. The unit test should fail without 
the patch, and fail with the patch. Then create a new patch including the patch.

If it's tricky to write a reliable test reproducing the race condition, you can 
use unittest.mock to mock some objects.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Glenn Langford]

Glenn Langford added the comment:

Proposed patch for as_completed(). #20369 fixes wait(), and behaviour is 
consistent between the two.

--
keywords: +patch
Added file: http://bugs.python.org/file33684/issue20367.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Guido van Rossum]

Guido van Rossum added the comment:

I think you the caller was wrong to pass in [f, f] in the first place.

In asyncio, the argument is converted into a set before using it, but there's 
still a bug if you pass it a list containing two references to the same 
coroutine -- it gets wrapped in two separate Futures. I think the better 
behavior is to convert to a set first and then map coroutines to Futures.

So concurrent.futures should also convert to a set first.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[STINNER Victor]

STINNER Victor added the comment:

Since the new asyncio module has also a Future class and functions like 
as_completed(), this issue concerns also asyncio. concurrent.futures and 
asyncio should have the same behaviour.

--
nosy: +gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Glenn Langford]

Glenn Langford added the comment:

There is a subtlety in the as_completed() code which explains a lot - note that 
"finished" starts off as a set in the _AcquireFutures block. So if a Future f 
has already completed,
   as_completed( [f,f] ) 
will only yield f once, because f appears once in the finished set.

Later on when waiter events are processed, "finished" turns into a list because 
of the line:

finished = waiter.finished_futures

So any duplicates in that list will cause problems in pending.remove(Future).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Glenn Langford]

New submission from Glenn Langford:

concurrent.futures.as_completed([f,f]) will yield f twice, then fail with a 
KeyError for a Future f which is not completed.

If the Future has already completed, as_completed([f,f]) will yield f once and 
does not trigger an exception.

What is the correct behaviour?
   as_completed( [f,f] ) -> yield f twice ?
   wait( [f,f], return_when=ALL_COMPLETED ) -> yield f twice ?

--
components: Library (Lib)
files: test_dupfuture.py
messages: 208952
nosy: glangford
priority: normal
severity: normal
status: open
title: concurrent.futures.as_completed() fails when given duplicate Futures
type: behavior
versions: Python 3.3, Python 3.4
Added file: http://bugs.python.org/file33658/test_dupfuture.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20367] concurrent.futures.as_completed() fails when given duplicate Futures

[Glenn Langford]

Changes by Glenn Langford :


--
nosy: +haypo, mark.dickinson, tim.peters

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com