python / python/cpython

Make concurrent iteration over pairwise, combinations, permutations, cwr, product, etc. from itertools safe under free-threading

Đang mở
#123,471 7 bình luận 0 reaction 1 người được giao Xem trên GitHub

@rhettinger đang làm issue này rồi.

Từ ngày 9/9/2024.

extension-modules sprint topic-free-threading type-bug
Ngôn ngữ chính
Python
Star
77.2k
Fork
35.9k
Chỉ số merge pull request
Chỉ số pull request đang chờ

Mô tả

Bug report

Bug description:

Several methods from the C implementation of the itertools module are not yet safe to use under the free-threading build. In this issue we list several issues to be addressed. The issues below are discussed for itertools.product, but the issues are similar for the other classes.

https://github.com/python/cpython/blob/58ce131037ecb34d506a613f21993cde2056f628/Modules/itertoolsmodule.c#L2038-L2044

This is not thread-safe, as multiple threads could have result == NULL evaluate to true. We could move the construction of the productobject.result to the constructor of product. This does mean that product will use more memory before the first invocation of next. This seems to be acceptable, as constructing a product without iterating over it seems rare in practice.
The tuple also needs to be filled with data. For product it seems safe to do this in the constructor, as the data is coming
from productobject->pools which is a tuple of tuples. But for pairwise the data is coming from an iterable

https://github.com/python/cpython/blob/58ce131037ecb34d506a613f21993cde2056f628/Modules/itertoolsmodule.c#L337-L343

which could be a generator. Reading data from the iterator before the first invocation of pairwise_next seems like a behavior change we do not want to make.

An alternative is to use some kind of locking inside product_next, but the locking should not add any overhead in the common path otherwise the single-thread performance will suffer.

  • In case iterables are exhausted some cleaning up is done. For example in pairwise_next at

https://github.com/python/cpython/blob/58ce131037ecb34d506a613f21993cde2056f628/Modules/itertoolsmodule.c#L352-L356

This cleaning up is not safe in concurrent iteration. Instead we can defer the cleaning up untill the object itself is decallocated (this approach was used for reversed, see https://github.com/python/cpython/pull/120971/files#r1653313765)

  • Actually constructing the new result requires some care as well. Even if we are fine with having funny results under concurrent iteration (see the discussion https://github.com/python/cpython/issues/120496), the concurrent iteration should not corrupt the interpreter. For example this code is not safe:

https://github.com/python/cpython/blob/58ce131037ecb34d506a613f21993cde2056f628/Modules/itertoolsmodule.c#L2077-L2088

If two threads both increment indices[i] the check on line 2078 is never true end we end up indexing pool with PyTuple_GET_ITEM outside the bounds on line 2088. Here we could change the check into indices[i] >= PyTuple_GET_SIZE(pool). That is equivalent for the single-threaded case, but does not lead to out-of-bounds indexing in the multi-threaded case (although it does lead to funny results!)

@rhettinger @colesbury Any input on the points above would be welcome.

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs
  • gh-123848
  • gh-125417
  • gh-129416
  • gh-131212
  • gh-131247
  • gh-132814
  • gh-135689
  • gh-144402
  • gh-144486
  • gh-144489
  • gh-144528
  • gh-146021
  • gh-146033
  • gh-148348
  • gh-153791

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.