| msg118416 - (view) |
Author: Hirokazu Yamamoto (ocean-city) *  |
Date: 2010-10-12 11:57 |
| Currently, PC/winreg.c releases GIL while calling registry API, but I found this in Remarks section of RegEnumValue. http://msdn.microsoft.com/en-us/library/ms724865%28VS.85%29.aspx > While using RegEnumValue, an application should not call any registry > functions that might change the key being queried. Maybe we shouldn't release GIL in PC/winreg.c? Thank you. |
|
|
| msg118493 - (view) |
Author: Brian Curtin (brian.curtin) * |
Date: 2010-10-13 00:35 |
| Some of your submission appears to have gotten lost (I saw it via email). Below is the patch you proposed. I haven't experienced this crash on my machines, but the solution seems fine to me. # I sometimes experienced crash of test_changing_value(test_winreg) # on release27-maint. It happens when 2 threads calls PyEnumValue and # PySetValue simultaneously. And I could stop it by following patch. # I'll attach the stack trace of crash. Index: PC/_winreg.c =================================================================== --- PC/_winreg.c (revision 85344) +++ PC/_winreg.c (working copy) @@ -1219,7 +1219,6 @@ } while (1) { - Py_BEGIN_ALLOW_THREADS rc = RegEnumValue(hKey, index, retValueBuf, @@ -1228,7 +1227,6 @@ &typ, (BYTE *)retDataBuf, &retDataSize); - Py_END_ALLOW_THREADS if (rc != ERROR_MORE_DATA) break; @@ -1577,9 +1575,7 @@ if (subKey == NULL) return NULL; } - Py_BEGIN_ALLOW_THREADS rc = RegSetValue(hKey, subKey, REG_SZ, str, len+1); - Py_END_ALLOW_THREADS if (rc != ERROR_SUCCESS) return PyErr_SetFromWindowsErrWithFunction(rc, "RegSetValue"); Py_INCREF(Py_None); |
|
|
| msg118499 - (view) |
Author: Daniel Stutzbach (stutzbach) |
Date: 2010-10-13 02:43 |
| Hirokazu, could you run Python in a debugger and figure out exactly which line crashes and what the error is? I'm curious. If we make this change, the same change should be applied to RegEnumKey, etc., since the RegEnumKey docs contain similar language. |
|
|
| msg118508 - (view) |
Author: Hirokazu Yamamoto (ocean-city) * |
Date: 2010-10-13 09:55 |
| One thread is exactly on RegEnumValue or RegQueryValue, and another thread is a bit after RegSetValue (the place varies case by case). Type is SEGV. |
|
|
| msg192649 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2013-07-08 13:11 |
| LGTM I suggest that you add a comment with a link to this issue. People may wonder why some places don't release the GIL. |
|
|
| msg224597 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-08-03 00:06 |
| Apart from the request for a comment made in it looks as if this can be commited. |
|
|
| msg224732 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2014-08-04 15:21 |
| I don't think this is an appropriate fix, since in most cases there is no need to prevent other Python threads running while inside RegSetValue. There are also other ways that a context switch may occur during the enumeration which will put the program in exactly the same state. (It isn't clear from the patch, but the two sections of changed code are from completely different functions.) The correct fix should be at the user's application level, or in a higher-level module than _winreg is supposed to be. |
|
|
| msg404253 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2021-10-19 00:11 |
| With Steve's opposition and the fact that we've gotten along for 11 years since this issue was opened without it (and also without further reports of issues), I'm going to go ahead and close the issue. |
|
|