-
Notifications
You must be signed in to change notification settings - Fork 762
Modernize import hook #1369
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
Modernize import hook #1369
Changes from 1 commit
a321daa
279b535
f92e95b
afffc18
d821c0f
e469a8a
685b972
be81364
73958ed
e71a0ef
2af066d
bb490bf
31ea876
c02d5c6
970a189
059605b
ff170e9
63de923
624f7e3
bd7e745
46a85fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Don't piggyback on AssemblyManager's AssemblyLoadHandler method because it may be called from other threads, leading to deadlocks if it is called while Python code is executing
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -176,30 +176,21 @@ static void SetupNamespaceTracking() | |
newset.Dispose(); | ||
} | ||
|
||
// AssemblyManager.namespaceAdded += OnNamespaceAdded; | ||
// PythonEngine.AddShutdownHandler(() => AssemblyManager.namespaceAdded -= OnNamespaceAdded); | ||
} | ||
|
||
/// <summary> | ||
/// Removes the set of available namespaces from the clr module and | ||
/// removes the callback on the OnNamespaceAdded event. | ||
/// Removes the set of available namespaces from the clr module. | ||
/// </summary> | ||
static void TeardownNameSpaceTracking() | ||
{ | ||
// AssemblyManager.namespaceAdded -= OnNamespaceAdded; | ||
// If the C# runtime isn't loaded, then there are no namespaces available | ||
Runtime.PyDict_SetItemString(root.dict, availableNsKey, Runtime.PyNone); | ||
} | ||
|
||
public static void OnNamespaceAdded(string name) | ||
public static void AddNamespace(string name) | ||
{ | ||
Console.WriteLine(System.Environment.StackTrace); | ||
Console.WriteLine("OnNamespaceAdded: acquiring"); | ||
Console.Out.Flush(); | ||
using (Py.GIL()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there scenarios when this method will be called without GIL acquired? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It used to be possible. The method calling it now has the attribute |
||
{ | ||
Console.WriteLine("OnNamespaceAdded: acquired"); | ||
Console.Out.Flush(); | ||
var pyNs = Runtime.PyString_FromString(name); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: |
||
try | ||
{ | ||
|
@@ -217,8 +208,6 @@ public static void OnNamespaceAdded(string name) | |
Runtime.XDecref(pyNs); | ||
} | ||
} | ||
Console.WriteLine("OnNamespaceAdded: released"); | ||
Console.Out.Flush(); | ||
} | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not remove the key entirely? You would not need to compare it to
None
below in that case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DotNetFinder relies on that key being present on the object. If I remove it in AddNamespace, I need to add a check over there instead.