Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

10 MB write to stream crashes Python #20

Closed
wolfmanstout opened this issue Nov 20, 2022 · 6 comments
Closed

10 MB write to stream crashes Python #20

wolfmanstout opened this issue Nov 20, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@wolfmanstout
Copy link

Simple repro on Windows 11 Python 3.9 64-bit:

import winsdk.windows.storage.streams as streams
data_writer = streams.DataWriter()
data_writer.write_bytes(10000000 * [0])
print("Wrote data")

This works fine with winrt. Try doing 100 MB if that doesn't repro.

Event Viewer log:

Faulting application name: python.exe, version: 3.9.12150.1013, time stamp: 0x623bb3b2
Faulting module name: ucrtbase.dll, version: 10.0.22621.608, time stamp: 0xf5fc15a3
Exception code: 0xc0000005
Fault offset: 0x0000000000051370
Faulting process id: 0x0x3154
Faulting application start time: 0x0x1D8FCF6FC83A24B
Faulting application path: C:\Python39-64\python.exe
Faulting module path: C:\WINDOWS\System32\ucrtbase.dll
Report Id: ebaa31ca-9228-41f8-bb92-e7659f0879c2
Faulting package full name: 
Faulting package-relative application ID: 
@dlech
Copy link
Contributor

dlech commented Nov 28, 2022

FYI, this will be more efficient (and maybe avoid the crash?) if an object that supports the buffer protocol is passed instead of a list.

data_writer.write_bytes(10000000 * b'\x00')

But we should still fix the crash.

@dlech
Copy link
Contributor

dlech commented Nov 28, 2022

Bug was introduce by pywinrt/pywinrt@9564a99. Returning com_array as array_view apparently frees the underlying buffer since array_view doesn't have a move constructor.

@dlech dlech added the bug Something isn't working label Nov 28, 2022
@wolfmanstout
Copy link
Author

Thank you for looking into this! FWIW, winrt actually doesn't allow passing in a bytes object as you suggest (although it does seem to resolve the issue with winsdk). winrt gives the following error:

>>> data_writer.write_bytes(10000000 * b'\x00')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: The parameter is incorrect.

I only mention this in case the goal is to provide an identical drop-in replacement for winrt.

dlech added a commit to pywinrt/pywinrt that referenced this issue Dec 11, 2022
This changes generated code for *pass array* parameters to only allow
objects that implement the Python buffer protocol.

This also fixes freeing com_array before use.

Issue: pywinrt/python-winsdk#20
@wolfmanstout
Copy link
Author

Hi, when is your ETA on pushing out a new release with the fix? I had another bug filed from a user of my package concerned about winrt being unmaintained, and this is the only blocker for me to switch over. Thanks!

@dlech
Copy link
Contributor

dlech commented Feb 9, 2023

I've been putting off doing a release because there have been some breaking changes and I haven't decided yet if the tradeoffs of the performance improvements are worth the breaking changes yet or not. Hopefully I can find some time to come back to this again soonish.

@dlech
Copy link
Contributor

dlech commented Apr 2, 2023

Should be fixed in v1.0.0b8. Note that write_bytes() now takes a bytes-like object instead of a list.

@dlech dlech closed this as completed Apr 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants