andrewfayres commented on issue #11885: Fix JNI custom op code from deregistering the operator fixes #10438 URL: https://github.com/apache/incubator-mxnet/pull/11885#issuecomment-409332419 Adding a bit of detail based on our conversations this morning @lanking520 I've seen no memory leaks resulting from the counter being removed. The reason is because the code that would eventually run after getting passed the counter would clean up very little. It'd remove the entry from the globalOperator map and delete the reference in the jvm. This effectively deregistered the operator in the JNI side while it could still be in scope in the scala side. The memory that leaks here is gets allocated during the Operator.register scala call. This is the same before and after this fix (we should add a dispose method and/or a Operator.deregister method but that's a bit beyond the scope of fixing this bug). @nswamy I do not believe that there are currently any existing unit tests for our custom operator jni code. The whole JNI file should be tested via unit tests and I will create an issue for us to follow through on that. We do have a custom operator example that gets run during the CI and provides a minimal amount of testing. My manual testing mostly involved running a modified version of our example multiple times. Verifying that I could reproduce the bug and that the crash stopped happening after digging into the code and make the change. Then I verified that both my modified version and the original still worked after the change. The code that was being run after the count check was guaranteed to make an operator unusable in the native side and would crash whenever the jvm tried to use that operator.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
