Skip to content

Assemblymanager thread safety #277

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.

Sign up for GitHub

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

Merged
filmor merged 14 commits into from
Nov 17, 2016
Merged

Assemblymanager thread safety #277

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
89a5504
Use ConcurrentDictionary for thread safety of import clr
Oct 26, 2016
cd1b9b9
Add thread safety test, use ConcurrentDictionary, lock for assemblies
Oct 27, 2016
596c210
Use atomic TryAdd call here as well
Oct 27, 2016
b9e532d
Print System.__file__ if assertion fails
Oct 27, 2016
2742b12
Wrap assemblies in thread-safe class, make sure forked thread joins i…
Oct 27, 2016
6838be8
No need to support recursion, also lock when we get _list.Count
Oct 28, 2016
59b9ea1
Import some .NET types as well and use them
Oct 28, 2016
2f416c4
Part of the System namespace is implemented in mscorlib.dll
Oct 28, 2016
3844d19
Missed an asterisk
Oct 28, 2016
c8ddb63
Merge branch 'master' into assemblymanager_thread_safety
Oct 30, 2016
bce86dc
Grab lock *before* getting enumerator
Nov 4, 2016
f3f7151
Merge branch 'assemblymanager_thread_safety' of https://github.com/Ar…
Nov 4, 2016
abb5eff
Merge branch 'master' into assemblymanager_thread_safety
ArvidJB Nov 14, 2016
cee2eb6
Merge branch 'master' into assemblymanager_thread_safety
filmor Nov 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Print System.__file__ if assertion fails
  • Loading branch information
abessen committed Oct 27, 2016
commit b9e532d800177054dc2504e5669fe07bb4e59e2b
3 changes: 2 additions & 1 deletion src/tests/test_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ def testModuleInterface(self):
self.assertEquals(type(System.__dict__), type({}))
self.assertEquals(System.__name__, 'System')
# the filename can be any module from the System namespace (eg System.Data.dll or System.dll)
self.assertTrue(fnmatch(System.__file__, "*System*.dll"))
self.assertTrue(fnmatch(System.__file__, "*System*.dll"),
"unexpected System.__file__" + System.__file__)
Copy link
Copy Markdown
Contributor

@den-run-ai den-run-ai Oct 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ArvidJB can you add a comment why this was added in test?

Copy link
Copy Markdown
Contributor Author

@ArvidJB ArvidJB Oct 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The System namespace is spread out across a couple of DLLs: mscorlib.dll, System.dll, and all the other System.*.dlls. In ModuleObject we iterate through all the known assemblies for a namespace and pick the last one for the filename. The iteration order would be the on the hash of the assembly, so it should have always been unpredictable whether you get mscorlib.dll or one of the others.
Let me change the test such that mscorlib.dll is also accepted.

Copy link
Copy Markdown
Contributor Author

@ArvidJB ArvidJB Oct 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS: I added the error message because I could not reproduce this on my machine and wanted to know what value it got in the integration tests.

self.assertTrue(System.__doc__.startswith("Namespace containing types from the following assemblies:"))
self.assertTrue(self.isCLRClass(System.String))
self.assertTrue(self.isCLRClass(System.Int32))
Expand Down