-
Notifications
You must be signed in to change notification settings - Fork 832
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
fix: Possible multithreading issue when two scores may come in parallel they may not safely fill pointer values #799
Conversation
…r changes. By boosterHandler not being serializable, it forces Serializable objects to declare it as transient
…rder of finalization so it is dangerous. I wonder how is it guaranteed that the memory will be cleared though
@JoanFM it looks like this needs to be merged with latest? I think I also see some code that has been merged already in the other PR. |
Yes, I am merging it |
|
||
private def getNumClasses: Int = { | ||
val numClassesOut = lightgbmlib.new_intp() | ||
LightGBMUtils.validate( | ||
lightgbmlib.LGBM_BoosterGetNumClasses(boosterPtr, numClassesOut), | ||
"Booster NumClasses") | ||
lightgbmlib.intp_value(numClassesOut) | ||
val out = lightgbmlib.intp_value(numClassesOut) | ||
lightgbmlib.delete_intp(numClassesOut) |
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.
nice catch to delete the pointer here!
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Codecov Report
@@ Coverage Diff @@
## master #799 +/- ##
=========================================
Coverage ? 43.83%
=========================================
Files ? 241
Lines ? 9704
Branches ? 529
=========================================
Hits ? 4254
Misses ? 5450
Partials ? 0
Continue to review full report at Codecov.
|
…el they may not safely fill pointer values (microsoft#799) * Move transient var to transient lazy val * Try guaranteeing NativeLibrary is loaded everytime it is needed but only loaded once * Forgot some lightGBMlibConstants * Add freeing of resources, handle all memory in BoosterHandler * Remove serialization from BoosterHandler, some documentation and minor changes. By boosterHandler not being serializable, it forces Serializable objects to declare it as transient * Style forbigs implementing finalize method, Java does not guarantee order of finalization so it is dangerous. I wonder how is it guaranteed that the memory will be cleared though * Add finalize method to call free for Native C++ allocated memory * predict normal correct constant * Change implementation to a ThreadLocal based synchronization
fix: Possible multithreading issue when two scores may come in parallel they may not safely fill pointer values
Two threads can fill up scoredDataOutPtr in parallel and mix up their values