You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/11/15 20:02:37 UTC

[GitHub] [beam] Abacn opened a new issue, #24181: [Task]: Eliminate finalize method (overrides Object.finalize) throughout Java SDK

Abacn opened a new issue, #24181:
URL: https://github.com/apache/beam/issues/24181

   ### What needs to happen?
   
   finalize() has been deprecated in Java 9 and has defects and per #24180 it is suspected that its usages might be cause of resource leak related bugs. We should remove its usage whenever possible.
   
   ### Issue Priority
   
   Priority: 2
   
   ### Issue Component
   
   Component: sdk-java-core


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] dspasic commented on issue #24181: [Task]: Eliminate deprecated finalize method (overrides Object.finalize) throughout Java SDK

Posted by "dspasic (via GitHub)" <gi...@apache.org>.
dspasic commented on issue #24181:
URL: https://github.com/apache/beam/issues/24181#issuecomment-1414103622

   Hi @Abacn ,
   
   do you have a suggestion on how to proceed with the task? As we have seen, the remaining finalize methods are not so easy to replace. I am also not sure if they should be removed in version 2. Even if the methods are not public.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] DhirajSardar commented on issue #24181: [Task]: Eliminate deprecated finalize method (overrides Object.finalize) throughout Java SDK

Posted by GitBox <gi...@apache.org>.
DhirajSardar commented on issue #24181:
URL: https://github.com/apache/beam/issues/24181#issuecomment-1361190386

   Hey I am newbie ?  Can I work on this issue ?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] dspasic commented on issue #24181: [Task]: Eliminate deprecated finalize method (overrides Object.finalize) throughout Java SDK

Posted by GitBox <gi...@apache.org>.
dspasic commented on issue #24181:
URL: https://github.com/apache/beam/issues/24181#issuecomment-1362250796

   Hi @DhirajSardar,
   
   I already replaced all finalize methods with the Teardown Context. At least the ones that could be replaced with the Teardown Context. 
   
   However, as far as I can tell, we still have 7 finalize methods left that I don't think can be replaced with the Teardown Context.
   
   ```
    ± grep -isr 'void finalize(' ./sdks/java
   ./sdks/java/core/src/main/java/org/apache/beam/sdk/io/Read.java:      protected void finalize() throws Throwable {
   ./sdks/java/io/amazon-web-services2/src/test/java/org/apache/beam/sdk/io/aws2/StaticSupplier.java:  protected void finalize() {
   ./sdks/java/io/jms/src/main/java/org/apache/beam/sdk/io/jms/JmsIO.java:    protected void finalize() {
   ./sdks/java/io/amazon-web-services/src/test/java/org/apache/beam/sdk/io/aws/dynamodb/StaticAwsClientsProvider.java:  protected void finalize() {
   ./sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/ReadFromKafkaDoFn.java:    protected void finalize() {
   ./sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsublite/internal/CloserReference.java:  protected void finalize() {
   ./sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsublite/internal/MemoryLimiterImpl.java:    public void finalize() {
   ```
   
   I must admit that it is not entirely clear how to proceed here. As far as I can judge after the first look, you can't replace them without further ado. [BoundedSourceAsSDFRestrictionTracker](https://github.com/apache/beam/blob/c627e472bd727936ff15da7bbbd60e6d48c13680/sdks/java/core/src/main/java/org/apache/beam/sdk/io/Read.java#L393), for example, is returned in [BoundedSourceAsSDFWrapperFn(::restrictionTracker)](https://github.com/apache/beam/blob/c627e472bd727936ff15da7bbbd60e6d48c13680/sdks/java/core/src/main/java/org/apache/beam/sdk/io/Read.java#L531), and the class is used in [Read::expand](https://github.com/apache/beam/blob/c627e472bd727936ff15da7bbbd60e6d48c13680/sdks/java/core/src/main/java/org/apache/beam/sdk/io/Read.java#L143). 
   
   My suggestion, in this case, would be to add a teardown context to BoundedSourceAsSDFWrapperFn. Also, BoundedSourceAsSDFWrapperFn must remember all created BoundedSourceAsSDFRestrictionTracker, which will be closed with the teardown context.
   
   Maybe @Abacn has some advice on how to proceed with the issue.
   
   To get back to your question. From my point of view, you are welcome to have a look at the other classes. If you decide to do so, we should ensure that we are not working on the same class in parallel. How does that sound to you?
   Looking forward to your feedback.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] chamikaramj commented on issue #24181: [Task]: Eliminate deprecated finalize method (overrides Object.finalize) throughout Java SDK

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on issue #24181:
URL: https://github.com/apache/beam/issues/24181#issuecomment-1315913553

   Close this ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] Abacn commented on issue #24181: [Task]: Eliminate deprecated finalize method (overrides Object.finalize) throughout Java SDK

Posted by GitBox <gi...@apache.org>.
Abacn commented on issue #24181:
URL: https://github.com/apache/beam/issues/24181#issuecomment-1316027525

   > Close this ?
   
   @chamikaramj  Besides https://github.com/apache/beam/pull/24180 there are still other appearence of this method throughout Java SDK so leave this issue open
   
   PS: Sorry I thought I chose "quote apply" but apparently edited your comment. Scary part having write access to the repo...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] itsarraj commented on issue #24181: [Task]: Eliminate deprecated finalize method (overrides Object.finalize) throughout Java SDK

Posted by "itsarraj (via GitHub)" <gi...@apache.org>.
itsarraj commented on issue #24181:
URL: https://github.com/apache/beam/issues/24181#issuecomment-1411384739

   Hello sir ,
   I am new to open source contribution.
   I already know java , my tech stacks & tools includes C, C++ , Python , Java, JavaScript , HTML , CSS , SQL , Bootstrap, ReactJS, ExpressJS, NodeJS & Git . I need a little help from your side to contribute to these amazing projects. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] dspasic commented on issue #24181: [Task]: Eliminate deprecated finalize method (overrides Object.finalize) throughout Java SDK

Posted by GitBox <gi...@apache.org>.
dspasic commented on issue #24181:
URL: https://github.com/apache/beam/issues/24181#issuecomment-1329732174

   .take-issue


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] Abacn commented on issue #24181: [Task]: Eliminate deprecated finalize method (overrides Object.finalize) throughout Java SDK

Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on issue #24181:
URL: https://github.com/apache/beam/issues/24181#issuecomment-1412221471

   @itsarraj Thanks for your interest. I put this as "good first issue" because I think it is a good way to get familiar with DoFn lifecycle. Some existing appearance of finalize is already eliminated since then. There are some other appearance looks not trivial (e.g. involving range trackers). Here I propose removing that tag to avoid confusing our potential contributors.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] Abacn commented on issue #24181: [Task]: Eliminate deprecated finalize method (overrides Object.finalize) throughout Java SDK

Posted by GitBox <gi...@apache.org>.
Abacn commented on issue #24181:
URL: https://github.com/apache/beam/issues/24181#issuecomment-1362291322

   Thanks @dspasic for the detailed investigation. If it is a DoFn then the same approach can be used (finalize -> tearDown) as @dspasic already did. If not I haven't look into the alternatives. Note that DoFn.tearDown is also best effort, anyways. Also, finalize is deprecated but there is no plan to remove it according to java documentation.
   
   @DhirajSardar thank you for the interests. There is a [good first issue](https://github.com/apache/beam/labels/good%20first%20issue) label where you can find some issues that interests you.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org