Browse files

Fix for cp33720

 - lock non-threadsafe LRU cache before use.

Add test for cp33720
  • Loading branch information...
1 parent c58e035 commit 8772fd3489bf77cfe92b1d78f1e3a4218c4a1bd7 @mvanderkolff mvanderkolff committed Feb 8, 2013
View
77 Languages/IronPython/IronPython.Modules/_struct.cs
@@ -95,7 +95,11 @@ public class Struct : IWeakReferenceable {
_formatString = fmt;
Struct s;
- if (_cache.TryGetValue(_formatString, out s)) {
+ bool gotIt;
+ lock (_cache) {
+ gotIt = _cache.TryGetValue(_formatString, out s);
+ }
+ if (gotIt) {
Initialize(s);
} else {
Compile(context, fmt);
@@ -536,8 +540,9 @@ public class Struct : IWeakReferenceable {
_encodingSize += GetNativeSize(_formats[i].Type) * _formats[i].Count;
}
-
- _cache.Add(fmt, this);
+ lock (_cache) {
+ _cache.Add(fmt, this);
+ }
}
#endregion
@@ -641,90 +646,66 @@ private struct Format {
private const int MAX_CACHE_SIZE = 1024;
private static CacheDict<string, Struct> _cache = new CacheDict<string, Struct>(MAX_CACHE_SIZE);
+ private static Struct GetStructFromCache(CodeContext/*!*/ context, [NotNull] string fmt/*!*/) {
+ Struct s;
+ bool gotIt;
+ lock (_cache) {
+ gotIt = _cache.TryGetValue(fmt, out s);
+ }
+ if (!gotIt) {
+ s = new Struct(context, fmt);
+ }
+ return s;
+ }
+
[Documentation("Clear the internal cache.")]
public static void _clearcache() {
_cache = new CacheDict<string, Struct>(MAX_CACHE_SIZE);
}
[Documentation("int(x[, base]) -> integer\n\nConvert a string or number to an integer, if possible. A floating point\nargument will be truncated towards zero (this does not include a string\nrepresentation of a floating point number!) When converting a string, use\nthe optional base. It is an error to supply a base when converting a\nnon-string. If base is zero, the proper base is guessed based on the\nstring content. If the argument is outside the integer range a\nlong object will be returned instead.")]
public static int calcsize(CodeContext/*!*/ context, [NotNull]string fmt) {
- Struct s;
- if (!_cache.TryGetValue(fmt, out s)) {
- s = new Struct(context, fmt);
- }
- return s.size;
+ return GetStructFromCache(context, fmt).size;
}
[Documentation("Return string containing values v1, v2, ... packed according to fmt.")]
public static string/*!*/ pack(CodeContext/*!*/ context, [NotNull]string fmt/*!*/, params object[] values) {
- Struct s;
- if (!_cache.TryGetValue(fmt, out s)) {
- s = new Struct(context, fmt);
- }
- return s.pack(context, values);
+ return GetStructFromCache(context, fmt).pack(context, values);
}
[Documentation("Pack the values v1, v2, ... according to fmt.\nWrite the packed bytes into the writable buffer buf starting at offset.")]
public static void pack_into(CodeContext/*!*/ context, [NotNull]string/*!*/ fmt, [NotNull]ArrayModule.array/*!*/ buffer, int offset, params object[] args) {
- Struct s;
- if (!_cache.TryGetValue(fmt, out s)) {
- s = new Struct(context, fmt);
- }
- s.pack_into(context, buffer, offset, args);
+ GetStructFromCache(context, fmt).pack_into(context, buffer, offset, args);
}
[Documentation("Unpack the string containing packed C structure data, according to fmt.\nRequires len(string) == calcsize(fmt).")]
public static PythonTuple/*!*/ unpack(CodeContext/*!*/ context, [NotNull]string/*!*/ fmt, [NotNull]string/*!*/ @string) {
- Struct s;
- if (!_cache.TryGetValue(fmt, out s)) {
- s = new Struct(context, fmt);
- }
- return s.unpack(context, @string);
+ return GetStructFromCache(context, fmt).unpack(context, @string);
}
[Documentation("Unpack the string containing packed C structure data, according to fmt.\nRequires len(string) == calcsize(fmt).")]
public static PythonTuple/*!*/ unpack(CodeContext/*!*/ context, [NotNull]string/*!*/ fmt, [NotNull]ArrayModule.array/*!*/ buffer) {
- Struct s;
- if (!_cache.TryGetValue(fmt, out s)) {
- s = new Struct(context, fmt);
- }
- return s.unpack(context, buffer);
+ return GetStructFromCache(context, fmt).unpack(context, buffer);
}
[Documentation("Unpack the string containing packed C structure data, according to fmt.\nRequires len(string) == calcsize(fmt).")]
public static PythonTuple/*!*/ unpack(CodeContext/*!*/ context, [NotNull]string fmt/*!*/, [NotNull]PythonBuffer/*!*/ buffer) {
- Struct s;
- if (!_cache.TryGetValue(fmt, out s)) {
- s = new Struct(context, fmt);
- }
- return s.unpack(context, buffer);
+ return GetStructFromCache(context, fmt).unpack(context, buffer);
}
[Documentation("Unpack the buffer, containing packed C structure data, according to\nfmt, starting at offset. Requires len(buffer[offset:]) >= calcsize(fmt).")]
public static PythonTuple/*!*/ unpack_from(CodeContext/*!*/ context, [NotNull]string fmt/*!*/, [NotNull]ArrayModule.array/*!*/ buffer, [DefaultParameterValue(0)] int offset) {
- Struct s;
- if (!_cache.TryGetValue(fmt, out s)) {
- s = new Struct(context, fmt);
- }
- return s.unpack_from(context, buffer, offset);
+ return GetStructFromCache(context, fmt).unpack_from(context, buffer, offset);
}
[Documentation("Unpack the buffer, containing packed C structure data, according to\nfmt, starting at offset. Requires len(buffer[offset:]) >= calcsize(fmt).")]
public static PythonTuple/*!*/ unpack_from(CodeContext/*!*/ context, [NotNull]string fmt/*!*/, [NotNull]string/*!*/ buffer, [DefaultParameterValue(0)] int offset) {
- Struct s;
- if (!_cache.TryGetValue(fmt, out s)) {
- s = new Struct(context, fmt);
- }
- return s.unpack_from(context, buffer, offset);
+ return GetStructFromCache(context, fmt).unpack_from(context, buffer, offset);
}
[Documentation("Unpack the buffer, containing packed C structure data, according to\nfmt, starting at offset. Requires len(buffer[offset:]) >= calcsize(fmt).")]
public static PythonTuple/*!*/ unpack_from(CodeContext/*!*/ context, [NotNull]string fmt/*!*/, [NotNull]PythonBuffer/*!*/ buffer, [DefaultParameterValue(0)] int offset) {
- Struct s;
- if (!_cache.TryGetValue(fmt, out s)) {
- s = new Struct(context, fmt);
- }
- return s.unpack_from(context, buffer, offset);
+ return GetStructFromCache(context, fmt).unpack_from(context, buffer, offset);
}
#endregion
View
65 Languages/IronPython/Tests/test_struct_threadsafe.py
@@ -0,0 +1,65 @@
+#####################################################################################
+#
+# Copyright (c) Michael van der Kolff. All rights reserved.
+#
+# This source code is subject to terms and conditions of the Apache License, Version 2.0. A
+# copy of the license can be found in the License.html file at the root of this distribution. If
+# you cannot locate the Apache License, Version 2.0, please send an email to
+# ironpy@microsoft.com. By using this source code in any fashion, you are agreeing to be bound
+# by the terms of the Apache License, Version 2.0.
+#
+# You must not remove this notice, or any other, from this software.
+#
+#
+#####################################################################################
+
+##
+## Test whether struct.pack (& others) are threadsafe. For the purpose of this test,
+## the only thing we care about is that no exception is thrown.
+##
+
+from iptest.assert_util import *
+import struct
+from threading import Thread
+from random import shuffle
+
+struct_pack_args = [
+ ("<II", 5, 7),
+ (">II", 7, 9),
+ ("<HHH", 5, 7, 9),
+ (">HII", 5, 7, 9),
+ (">QQQ", 5, 7, 9),
+ ("<QQQ", 5, 7, 9),
+ (">QQQQQ", 3, 9, 5, 7, 9),
+ ("dd", 5., 7.9),
+ ("fd", 5., 7.9),
+ ("df", 5., 7.9),
+ ("ff", 5., 7.9),
+]
+
+class PackThread(Thread):
+ def __init__(self, group=None, target=None, name=None, args=(), kwargs={}):
+ self.retval = None
+ return super(PackThread, self).__init__(group, target, name, args, kwargs)
+
+ def run(self):
+ my_args = list(struct_pack_args)
+ shuffle(my_args)
+ try:
+ for i in xrange(100000):
+ for args in my_args:
+ struct.pack(*args)
+ except:
+ self.retval = False
+ return
+ self.retval = True
+
+def test_packs():
+ pack_threads = [PackThread() for i in xrange(10)]
+ for t in pack_threads:
+ t.start()
+ for t in pack_threads:
+ t.join()
+ Assert(all(t.retval for t in pack_threads), "struct.pack: Is not threadsafe")
+
+run_test(__name__)
View
11 Test/IronPython.tests
@@ -1445,6 +1445,17 @@
<NotParallelSafe>false</NotParallelSafe>
<WorkingDirectory>%DLR_ROOT%\Languages\IronPython\Tests</WorkingDirectory>
</Test>
+ <Test>
+ <Name>test_struct_threadsafe_20</Name>
+ <Filename>%DLR_ROOT%\Languages\IronPython\Internal\ipy.bat</Filename>
+ <Arguments>test_struct_threadsafe.py</Arguments>
+ <MaxDuration>1200000</MaxDuration>
+ <LongRunning>true</LongRunning>
+ <Disabled>false</Disabled>
+ <RequiresAdmin>false</RequiresAdmin>
+ <NotParallelSafe>false</NotParallelSafe>
+ <WorkingDirectory>%DLR_ROOT%\Languages\IronPython\Tests</WorkingDirectory>
+ </Test>
</Tests>
</TestCategory>
<TestCategory>

0 comments on commit 8772fd3

Please sign in to comment.