-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Fixing issues with multi-thread execution #5187
base: dev
Are you sure you want to change the base?
Conversation
closes #5165
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.
Merge conflict
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.
lgtm !
suggested one change about closing shared resources, apart from that i think its time for us to work on possibly removing shared global singletons so i have created a issue to track it #5239
} | ||
defer tmpEngine.closeInternal() | ||
// create ephemeral nuclei objects/instances/types using base nuclei engine |
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.
@Mzack9999 , the tmpEngine uses shared resources like interactsh,outputwriter etc which are used by every nuclei instance created from threadsafeNucleiExecuter, while fixing memory leaks we accidently closed this here , but this is not expected to be closed here,
shared resources of threadsafeNucleiExecuter are released using .Close() of ThreadSafeNucleiEngine
as for temporary resources like ratelimit and stuff they are closed using defer closeEphemeralObjects(unsafeOpts)
i have removed this in my last pr and is included in latest, so we should remove this tmpEngine.closeInternal
Proposed changes
Fixing multiple issues during usage as SDK
Checklist