Implement cleaner to replace deprecated finalizers

[sabrinajlee]

The cleaner is the replacement for the deprecated finalize() method for cleaning objects before garbage collection.

Each class that was previously using finalize() will have an anonymous function that cleans up the necessary resources. The number of cleaner threads can be set by a property with the default value being 2. The object and its anonymous function are registered to one of the cleaner threads in round robin order.

Signed-off-by: Sabrina Lee [email protected]

[jasonkatonica]

I believe your DCO in the commit text and in the PR description should read:

Signed-off-by: Sabrina Lee <[email protected]>

Instead of

Signed-off by: Sabrina Lee <[email protected]>

Notice the - before the by above.

This might explain why the DCO check has failed.

[KostasTsiounis]

LGTM

[jasonkatonica]

I am a bit worried about printing stack traces on a mass scale if something goes wrong here but not sure we have any other options?

[sabrinajlee]

Would it make sense to limit how many times the stack trace prints with a counter inside the catch block? Since it could be more than a one time issue but if the cleaning is failing so frequently that it enters this catch block more than a certain number of times it will run into OOM so there should be a limit to how many times we think it needs to print.

[jasonkatonica]

That seems like a reasonable approach to me and better then any idea that i have at the moment. I assume we cannot throw an exception from this method at all?

[KostasTsiounis]

I think you can throw an exception just fine. The question is do you want to fail if it happens once?

[jasonkatonica]

I do think we should validate that this works ok in and around the max integer of 2,147,483,648

[sabrinajlee]

Tested out Math.floorMod() as well as resetting the count variable at Integer.MAX_VALUE and the latter seems like the better option because we can reset the value so that the next consecutive cleaner thread in the array is used, where the Math.floorMod() that depends on the number of cleaner threads and we can't control it. In either case the default number of cleaners (2) is fine but for a different number of cleaners the distribution of cleaners would become imbalanced over time.

[jasonkatonica]

OK seems reasonable to me to just reset the counter once we reach Integer.MAX_VALUE. I think the code will be more easy to understand also.

[KostasTsiounis]

The Math.floorMod() is not useful in integer calculations anyway. The check and resetting can be a bit time consuming, especially in an AtomicInteger. I don't see what the problem with using abs() is.

[jasonkatonica]

I think the problem is that abs() of Integer.MAX_VALUE + 1 returns a negative value.

[sabrinajlee]

abs() on Integer.MIN_VALUE returns a negative value, and then the % operator in Java works differently than in math, normally modulo should always return a positive number but in Java using % on a negative operand returns a negative value.

[KostasTsiounis]

If you're not gonna change anything in the thread, then the custom ThreadFactory is not needed at all and can be removed.