Fix multithreading issue #21
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| It doesn't look fixed. |
medegor44 force-pushed the ChakraCore_multithreading branch from f1a797f to 0947740 Compare 
confirmit-horizons approved these changes Oct 18, 2021
| var val = _engine.Evaluate(expression); | ||
| JsValue val; | ||
| | ||
| lock (_engineSynchronizer) |
There was a problem hiding this comment.
it would be better to remove the lock and mention in the readme that jint is not thread safe. then it will be up to the user to sync
| private readonly JsContext _context; | ||
| | ||
| // stores registered delegates to prevent their garbage collection | ||
| private readonly List<JavaScriptNativeFunction> _jsNativeFunctions = new(); |
There was a problem hiding this comment.
nit: you can use ConcurrentBag as you don't need a list with preserved order here
| return MapJsPrimitivesToClr(value); | ||
| t = MapJsPrimitivesToClr(value); | ||
| | ||
| return t; |
| var runner = new ChakraCoreRunner(); | ||
| Assert.Throws<ArgumentException>(() => runner.Register(new HelperObject(), "x")); | ||
| | ||
| runner.Dispose(); |
There was a problem hiding this comment.
nit: no need to explicit Dispose, can be inside using stmt
medegor44 force-pushed the ChakraCore_multithreading branch from 85d2d69 to b545e00 Compare Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Fix multithreading issue (only on windows)