Skip to content

Move fulfilling pending future outside the lock and ignore already completed futures#536

Merged
DmitriyMusatkin merged 14 commits intomainfrom
buffer_cancel
Jul 17, 2025
Merged

Move fulfilling pending future outside the lock and ignore already completed futures#536
DmitriyMusatkin merged 14 commits intomainfrom
buffer_cancel

Conversation

@DmitriyMusatkin
Copy link
Contributor

@DmitriyMusatkin DmitriyMusatkin commented Jul 15, 2025

Issue #, if available:

Description of changes:
Fulfilling pending future inside the lock can lead to deadlock if future is already canceled.
Add logic to ignore all canceled futures. And move future completion outside of lock (for the case where cancel occurs right after the future was checked)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2025

Codecov Report

❌ Patch coverage is 88.88889% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.51%. Comparing base (f8ae82e) to head (96794eb).
⚠️ Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
source/s3_default_buffer_pool.c 88.88% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #536   +/-   ##
=======================================
  Coverage   89.50%   89.51%           
=======================================
  Files          21       21           
  Lines        6567     6589   +22     
=======================================
+ Hits         5878     5898   +20     
- Misses        689      691    +2     
Files with missing lines Coverage Δ
source/s3_default_buffer_pool.c 95.66% <88.88%> (-0.46%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@DmitriyMusatkin DmitriyMusatkin merged commit 70aacd2 into main Jul 17, 2025
38 checks passed
@DmitriyMusatkin DmitriyMusatkin deleted the buffer_cancel branch July 17, 2025 06:22
passaro added a commit to passaro/mountpoint-s3 that referenced this pull request Jul 17, 2025
Update the `aws-c-s3` submodule to
[v0.8.5](https://github.com/awslabs/aws-c-s3/releases/tag/v0.8.5),
picking up in particular: [Move fulfilling pending future outside the lock and ignore already completed futures (awslabs#536)](awslabs/aws-c-s3#536).

Change details:
```
Submodule mountpoint-s3-crt-sys/crt/aws-c-s3 f8ae82e3..70aacd2d:
  > Move fulfilling pending future outside the lock and ignore already completed futures (awslabs#536)
```

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
passaro added a commit to passaro/mountpoint-s3 that referenced this pull request Jul 17, 2025
Update the `aws-c-s3` submodule to
[v0.8.5](https://github.com/awslabs/aws-c-s3/releases/tag/v0.8.5),
picking up in particular: [Move fulfilling pending future outside the lock and ignore already completed futures (awslabs#536)](awslabs/aws-c-s3#536).

Change details:
```
Submodule mountpoint-s3-crt-sys/crt/aws-c-s3 f8ae82e3..70aacd2d:
  > Move fulfilling pending future outside the lock and ignore already completed futures (awslabs#536)
```

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
github-merge-queue bot pushed a commit to awslabs/mountpoint-s3 that referenced this pull request Jul 17, 2025
Update the CRT submodules to the latest releases, picking up in
particular: [Move fulfilling pending future outside the lock and ignore
already completed futures
(#536)](awslabs/aws-c-s3#536).

<details>
  <summary>Full CRT changelog:</summary>

```
Submodule mountpoint-s3-crt-sys/crt/aws-c-common aaa2f11e..2b67a658:
  > Add API for a more compact (no dashes) UUID-to-str (#1212)
  > Add a python script to help pick up the latest cjson and libcbor (#1211)
  > Fix byte helpers for mingw 32 bit (#1210)
  > Remove Windows 2019 and add Windows 2025 with MSVC-17 (#1209)
  > Fix signature of aws_backtrace_log (#1206)
  > Remove clang-3 from CI (#1203)
Submodule mountpoint-s3-crt-sys/crt/aws-c-http 10961a70..bfa03928:
  > support no_proxy excatly like CURL (#522)
  > Remove Windows 2019 and add Windows 2025 with MSVC-17 (#521)
Submodule mountpoint-s3-crt-sys/crt/aws-c-io ee7925a3..12cb9f9c:
  > stop packing future variable to avoid tsan data race warnings (#741)
  > Support s2n security policy for TLS 1.2 and FIPS (#739)
Submodule mountpoint-s3-crt-sys/crt/aws-c-s3 f8ae82e3..70aacd2d:
  > Move fulfilling pending future outside the lock and ignore already completed futures (#536)
```
</details>

### Does this change impact existing behavior?

No.

### Does this change need a changelog entry? Does it require a version
change?

Client changelog.

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and I agree to the terms of
the [Developer Certificate of Origin
(DCO)](https://developercertificate.org/).

---------

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
passaro added a commit to passaro/mountpoint-s3 that referenced this pull request Jul 17, 2025
Update the CRT submodules to the latest releases, picking up in
particular: [Move fulfilling pending future outside the lock and ignore
already completed futures
(awslabs#536)](awslabs/aws-c-s3#536).

<details>
  <summary>Full CRT changelog:</summary>

```
Submodule mountpoint-s3-crt-sys/crt/aws-c-common aaa2f11e..2b67a658:
  > Add API for a more compact (no dashes) UUID-to-str (awslabs#1212)
  > Add a python script to help pick up the latest cjson and libcbor (awslabs#1211)
  > Fix byte helpers for mingw 32 bit (awslabs#1210)
  > Remove Windows 2019 and add Windows 2025 with MSVC-17 (awslabs#1209)
  > Fix signature of aws_backtrace_log (awslabs#1206)
  > Remove clang-3 from CI (awslabs#1203)
Submodule mountpoint-s3-crt-sys/crt/aws-c-http 10961a70..bfa03928:
  > support no_proxy excatly like CURL (awslabs#522)
  > Remove Windows 2019 and add Windows 2025 with MSVC-17 (awslabs#521)
Submodule mountpoint-s3-crt-sys/crt/aws-c-io ee7925a3..12cb9f9c:
  > stop packing future variable to avoid tsan data race warnings (awslabs#741)
  > Support s2n security policy for TLS 1.2 and FIPS (awslabs#739)
Submodule mountpoint-s3-crt-sys/crt/aws-c-s3 f8ae82e3..70aacd2d:
  > Move fulfilling pending future outside the lock and ignore already completed futures (awslabs#536)
```
</details>

### Does this change impact existing behavior?

No.

### Does this change need a changelog entry? Does it require a version
change?

Client changelog.

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and I agree to the terms of
the [Developer Certificate of Origin
(DCO)](https://developercertificate.org/).

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
passaro added a commit to passaro/mountpoint-s3 that referenced this pull request Jul 17, 2025
Update the CRT submodules to the latest releases, picking up in
particular: [Move fulfilling pending future outside the lock and ignore
already completed futures
(awslabs#536)](awslabs/aws-c-s3#536).

<details>
  <summary>Full CRT changelog:</summary>

```
Submodule mountpoint-s3-crt-sys/crt/aws-c-common aaa2f11e..2b67a658:
  > Add API for a more compact (no dashes) UUID-to-str (awslabs#1212)
  > Add a python script to help pick up the latest cjson and libcbor (awslabs#1211)
  > Fix byte helpers for mingw 32 bit (awslabs#1210)
  > Remove Windows 2019 and add Windows 2025 with MSVC-17 (awslabs#1209)
  > Fix signature of aws_backtrace_log (awslabs#1206)
  > Remove clang-3 from CI (awslabs#1203)
Submodule mountpoint-s3-crt-sys/crt/aws-c-http 10961a70..bfa03928:
  > support no_proxy excatly like CURL (awslabs#522)
  > Remove Windows 2019 and add Windows 2025 with MSVC-17 (awslabs#521)
Submodule mountpoint-s3-crt-sys/crt/aws-c-io ee7925a3..12cb9f9c:
  > stop packing future variable to avoid tsan data race warnings (awslabs#741)
  > Support s2n security policy for TLS 1.2 and FIPS (awslabs#739)
Submodule mountpoint-s3-crt-sys/crt/aws-c-s3 f8ae82e3..70aacd2d:
  > Move fulfilling pending future outside the lock and ignore already completed futures (awslabs#536)
```
</details>

### Does this change impact existing behavior?

No.

### Does this change need a changelog entry? Does it require a version
change?

Client changelog.

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and I agree to the terms of
the [Developer Certificate of Origin
(DCO)](https://developercertificate.org/).

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
mansi153 pushed a commit to mansi153/mountpoint-s3 that referenced this pull request Jul 24, 2025
Update the CRT submodules to the latest releases, picking up in
particular: [Move fulfilling pending future outside the lock and ignore
already completed futures
(awslabs#536)](awslabs/aws-c-s3#536).

<details>
  <summary>Full CRT changelog:</summary>

```
Submodule mountpoint-s3-crt-sys/crt/aws-c-common aaa2f11e..2b67a658:
  > Add API for a more compact (no dashes) UUID-to-str (awslabs#1212)
  > Add a python script to help pick up the latest cjson and libcbor (awslabs#1211)
  > Fix byte helpers for mingw 32 bit (awslabs#1210)
  > Remove Windows 2019 and add Windows 2025 with MSVC-17 (awslabs#1209)
  > Fix signature of aws_backtrace_log (awslabs#1206)
  > Remove clang-3 from CI (awslabs#1203)
Submodule mountpoint-s3-crt-sys/crt/aws-c-http 10961a70..bfa03928:
  > support no_proxy excatly like CURL (awslabs#522)
  > Remove Windows 2019 and add Windows 2025 with MSVC-17 (awslabs#521)
Submodule mountpoint-s3-crt-sys/crt/aws-c-io ee7925a3..12cb9f9c:
  > stop packing future variable to avoid tsan data race warnings (awslabs#741)
  > Support s2n security policy for TLS 1.2 and FIPS (awslabs#739)
Submodule mountpoint-s3-crt-sys/crt/aws-c-s3 f8ae82e3..70aacd2d:
  > Move fulfilling pending future outside the lock and ignore already completed futures (awslabs#536)
```
</details>

### Does this change impact existing behavior?

No.

### Does this change need a changelog entry? Does it require a version
change?

Client changelog.

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and I agree to the terms of
the [Developer Certificate of Origin
(DCO)](https://developercertificate.org/).

---------

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
dannycjones pushed a commit to c-hagem/mountpoint-s3 that referenced this pull request Oct 9, 2025
Update the CRT submodules to the latest releases, picking up in
particular: [Move fulfilling pending future outside the lock and ignore
already completed futures
(awslabs#536)](awslabs/aws-c-s3#536).

<details>
  <summary>Full CRT changelog:</summary>

```
Submodule mountpoint-s3-crt-sys/crt/aws-c-common aaa2f11e..2b67a658:
  > Add API for a more compact (no dashes) UUID-to-str (awslabs#1212)
  > Add a python script to help pick up the latest cjson and libcbor (awslabs#1211)
  > Fix byte helpers for mingw 32 bit (awslabs#1210)
  > Remove Windows 2019 and add Windows 2025 with MSVC-17 (awslabs#1209)
  > Fix signature of aws_backtrace_log (awslabs#1206)
  > Remove clang-3 from CI (awslabs#1203)
Submodule mountpoint-s3-crt-sys/crt/aws-c-http 10961a70..bfa03928:
  > support no_proxy excatly like CURL (awslabs#522)
  > Remove Windows 2019 and add Windows 2025 with MSVC-17 (awslabs#521)
Submodule mountpoint-s3-crt-sys/crt/aws-c-io ee7925a3..12cb9f9c:
  > stop packing future variable to avoid tsan data race warnings (awslabs#741)
  > Support s2n security policy for TLS 1.2 and FIPS (awslabs#739)
Submodule mountpoint-s3-crt-sys/crt/aws-c-s3 f8ae82e3..70aacd2d:
  > Move fulfilling pending future outside the lock and ignore already completed futures (awslabs#536)
```
</details>

### Does this change impact existing behavior?

No.

### Does this change need a changelog entry? Does it require a version
change?

Client changelog.

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and I agree to the terms of
the [Developer Certificate of Origin
(DCO)](https://developercertificate.org/).

---------

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants