[Foundation] Don't leak exceptions in WrappedNSInputStream.Read. by rolfbjarne · Pull Request #20131 · dotnet/macios

rolfbjarne · 2024-02-16T21:11:48Z

We don't want to leak exceptions back to the calling native code in WrappedNSInputStream.Read, because that will likely crash the process.

Example stack trace:

ObjectDisposed_StreamClosed (System.ObjectDisposedException)
   at System.ThrowHelper.ThrowObjectDisposedException_StreamClosed(String) + 0x3c
   at System.IO.MemoryStream.Read(Byte[], Int32, Int32) + 0x124
   at System.Net.Http.MultipartContent.ContentReadStream.Read(Byte[], Int32, Int32) + 0x78
   at System.Net.Http.NSUrlSessionHandler.WrappedNSInputStream.Read(IntPtr buffer, UIntPtr len) + 0x58
   at MyApp!<BaseAddress>+0x7082f8

Instead return -1 from the Read method, which is documented as an error
condition, and then also return a custom NSError from the Error property -
which is also documented to be where the error is supposed to be surfaced.

Ref: https://developer.apple.com/documentation/foundation/nsinputstream/1411544-read

Ref: #20123.

We don't want to leak exceptions back to the calling native code in WrappedNSInputStream.Read, because that will likely crash the process. Example stack trace: ObjectDisposed_StreamClosed (System.ObjectDisposedException) at System.ThrowHelper.ThrowObjectDisposedException_StreamClosed(String) + 0x3c at System.IO.MemoryStream.Read(Byte[], Int32, Int32) + 0x124 at System.Net.Http.MultipartContent.ContentReadStream.Read(Byte[], Int32, Int32) + 0x78 at System.Net.Http.NSUrlSessionHandler.WrappedNSInputStream.Read(IntPtr buffer, UIntPtr len) + 0x58 at MyApp!<BaseAddress>+0x7082f8 Instead return -1 from the Read method, which is documented as an error condition, and then also return a custom NSError from the Error property - which is also documented to be where the error is supposed to be surfaced. Ref: https://developer.apple.com/documentation/foundation/nsinputstream/1411544-read Ref: dotnet#20123.

…ont-leak-exceptions-in-nswrappedstream-read

vs-mobiletools-engineering-service2 · 2024-02-20T12:48:31Z

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2024-02-20T12:52:47Z

💻 [PR Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

✅ All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2024-02-20T12:52:53Z

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

✅ All tests on macOS M1 - Mac Ventura (13) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2024-02-20T12:53:33Z

💻 [CI Build] Windows Integration Tests passed 💻

✅ All Windows Integration Tests passed.

Pipeline on Agent
Hash: 4d87e78ff1d1d7492686edcb55b24cc88d7885d4 [PR build]

vs-mobiletools-engineering-service2 · 2024-02-20T12:54:30Z

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2024-02-20T12:55:55Z

💻 [PR Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

✅ All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: [PR build]

vs-mobiletools-engineering-service2 · 2024-02-20T13:27:07Z

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
watchOS: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
watchOS: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

.NET (No breaking changes)

iOS: vsdrops gist (No breaking changes)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)
Microsoft.iOS vs Microsoft.MacCatalyst: vsdrops gist

Legacy Xamarin (stable) vs .NET

iOS: vsdrops gist
tvOS: vsdrops gist
macOS: vsdrops gist

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 4d87e78ff1d1d7492686edcb55b24cc88d7885d4 [PR build]

vs-mobiletools-engineering-service2 · 2024-02-20T14:14:06Z

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 170 tests passed 🎉

Tests counts

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ install-source: All 1 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 7 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac-binding-project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 6 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 7 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 8 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (watchOS): All 4 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 4d87e78ff1d1d7492686edcb55b24cc88d7885d4 [PR build]

This comment has been minimized.

Sign in to view

tweaks

01206c1

This comment has been minimized.

Sign in to view

Merge remote-tracking branch 'origin/main' into nsurlsessionhandler-d…

05d8969

…ont-leak-exceptions-in-nswrappedstream-read

This comment has been minimized.

Sign in to view

Fix test.

4d87e78

rolfbjarne marked this pull request as ready for review February 20, 2024 15:49

rolfbjarne requested a review from mandel-macaque as a code owner February 20, 2024 15:49

dalexsoto approved these changes Feb 21, 2024

View reviewed changes

haritha-mohan approved these changes Feb 21, 2024

View reviewed changes

rolfbjarne merged commit 79ac366 into dotnet:main Feb 21, 2024

rolfbjarne deleted the nsurlsessionhandler-dont-leak-exceptions-in-nswrappedstream-read branch February 21, 2024 09:29

tipa mentioned this pull request Mar 7, 2024

Non-catchable ObjectDisposedException when using NSUrlSessionHandler #20123

Closed

Conversation

rolfbjarne commented Feb 16, 2024

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vs-mobiletools-engineering-service2 commented Feb 20, 2024

📚 [PR Build] Artifacts 📚

Packages generated

Uh oh!

vs-mobiletools-engineering-service2 commented Feb 20, 2024

💻 [PR Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Feb 20, 2024

💻 [PR Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Feb 20, 2024

💻 [CI Build] Windows Integration Tests passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Feb 20, 2024

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Feb 20, 2024

💻 [PR Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

Uh oh!

vs-mobiletools-engineering-service2 commented Feb 20, 2024

✅ API diff for current PR / commit

✅ API diff vs stable

ℹ️ Generator diff

Uh oh!

vs-mobiletools-engineering-service2 commented Feb 20, 2024

🚀 [CI Build] Test results 🚀

Test results

Tests counts

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants