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
_winreg.EnumValue sometimes raises WindowsError ("More data is available") #47059
Comments
_winreg.EnumValue raises a WindowsError ("More data is available") if Inspecting PyEnumValue in _winreg.c, I believe I see the problem. The Unfortunately, RegQueryInfoKey returns the size in number of unicode I believe it would be sufficient to multiply the sizes by 4, since The bug exists in at least Python 2.5 and Python 3.0 (based on source References: RegEnumValue: http://msdn.microsoft.com/en-us/library/ms724865(VS.85).aspx RegQueryInfoKey: |
Is that for Python 2.5 or 3.0? |
The bug is in both. On Sat, May 10, 2008 at 12:52 PM, Martin v. Löwis
|
Can you please provide a test case then? The 3.0 code doesn't use |
After several failed attempts at making a test case, and stepping I do still have a stack trace from an end-user of my python2.5-based The application reliably crashes on start-up for this user, when Unfortunately, I have been unable to reproduce the problem locally. I I'm going to try building a custom PyEnumValue that will dynamically In the meantime, I'm open to other theories on what might cause The end user is running Vista, if it matters. |
I suggest to use regedit /e to dump the failing key into a file. That |
To reproduce on Windows XP with Python 2.4, 2.5, and 2.6:
key = _winreg.OpenKey( _winreg.HKEY_CURRENT_USER, "PythonTestUnicodeKeys" )
one_byte_key = _winreg.EnumKey( key, 0 )
two_byte_key = _winreg.EnumKey( key, 1 )
_winreg.CloseKey( key ) try: print "should be unicode, not str:", two_byte_key.__class__ |
IIUC, no fix for this bug has been proposed, so the issue is no |
I propose to close this as "invalid", because the bug with _winreg.EnumValue can not be confirmed. However, it seems to be impossible to return unicode data from _winreg.EnumKey, and this deserves a new bug. |
Attached is a file that reliably produces the "More data is available" error from _winreg.EnumValue in Python 2.6. The script triggers the error via a race condition, by modifying the value after PyEnumValue() calls RegQueryInfoKey() but before it calls RegEnumValue(). I don't know if that's what caused the original problem for my end-user, but it certainly is one way to trigger the problem. I can work on a patch next week for a proper test case and to fix PyEnumValue(). |
Here's another script that causes a "More data is available" error. This one creates a key with a name that's exactly 256 characters long. (The Windows Registry Editor can't display the key either) I'm testing this on XP. Newer versions of Windows might handle long key names better, or have a different magic length that causes failures. |
If this doesn't relate to multibyte strings anymore, but just to long strings then I'd open new bug. If even regedit fails to query then maybe its WinAPI flaw? Maybe it will worth to try ctypes. |
It was never 100% clear that it ever related to multi-byte strings, so (as the original reporter) I'd prefer to continue using this bug and just update the title to: _winreg.EnumValue sometimes raises WindowsError ("More data is available"). The patch I have planned will fix the problem regardless of the cause, (except for impossible-to-fix causes, such as if there really is an API flaw). |
I just wrote a C program that can read the long key name just fine, so it's not a Windows API bug. |
I just found a one line example of the problem: >>> EnumValue(HKEY_PERFORMANCE_DATA, 0)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
WindowsError: [Error 234] More data is available Other functions are also affected: >>> QueryValueEx(HKEY_PERFORMANCE_DATA, None)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
WindowsError: [Error 234] More data is available In a nutshell, the Python implementation of these functions works like this:
However, the API functions called in step #1 are not guaranteed to be accurate. It works *most* of the time, but if step #3 returns ERROR_MORE_DATA then we need to dynamically grow the buffer and try again until the data fits. I'll have a patch (with test cases) ready later today. |
Great analysis! |
I've uploaded two patches (against trunk): one to add test cases that demonstrate this bug and another to fix the bug. |
test_changing_value is giving inconsistent results when the _winreg.c patch is not applied. It mostly fails on QueryValue, sometimes on EnumValue, and about 1/10 times the test does not fail at all. Ideally the tests should not use threads -- can the same thing be tested without a thread? It seems like the issue isn't related to concurrency, but maybe I missed something. With _winreg.c patched, the tests seem to pass, but I haven't run that as much as I have the unpatched version. winreg_test.pach
|
After a quick glance, the _winreg.c changes look ok. I'll try to fit in a review shortly. |
Thank for the feedback. I'll revise the patch tomorrow based on your code-cleanup suggestions. To answer your question about the thread and explain why the test sometimes passes: The goal of test_changing_value is to try to trigger a race condition that exists inside many of these function. So, yes, the thread is necessary. :) Thankfully the functions call Py_BEGIN_ALLOW_THREADS; otherwise, I'd have to launch another process to try to trigger it. Sometimes we never manage to hit the race condition and the test passes even though the bug is present. I can increase the number of attempts to trigger it (currently "for _ in range(100)") to make it more likely to occur every time the test is run. |
Attached is a new test-case patch. |
How much would increasing the loop count slow down the test? Since if the bug isn't present the loop will run to the end every time, if it is non-trivial it would probably be better to keep it shorter, since Brian said it failed one way or another 9 out of 10 times. Either way, a note in the test that it may occasionally pass even if the bug is present, but should fail most of the time, would be a good idea. |
In the updated patch I uploaded yesterday, I increased the loop count from 100 to 1000 and it still runs virtually instantly. I also added a comment stating that the test is trying to trigger a race condition. |
Committed to trunk in r81517 and release26-maint in r81540. I'll cover the 3.x stuff today and then close it out. |
test_dynamic_key fails on py3k, but not because of these changes. RegQueryValueExW doesn't appear to work with NULL for the second parameter (valueName), although it is documented to and the ANSI version on 2.x works fine. The empty string is also documented as an acceptable parameter, so while testing it out, I hardcoded "" in PyQueryValueEx and the calls worked. With NULL, it raises "WindowsError: [Error 87] The parameter is incorrect". Thoughts? The current py3k patch is attached. |
I wrote a short C program to test a few different variations. It looks to me like a bug in the operating system's implementation of HKEY_PERFORMANCE_DATA (which is a virtual registry key). If I pass NULL as the second parameter to a more conventional key that has a default value, everything works fine. I suggest changing the test to pass "" instead of None. |
Committed to py3k in r81547 and release31-maint in r81546. Thanks for the patch! |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: