Skip to content
This repository was archived by the owner on Aug 15, 2024. It is now read-only.

Removed deprecated method involving Thread#22

Closed
lavturo wants to merge 2 commits intophp:masterfrom
lavturo:fix_issue21
Closed

Removed deprecated method involving Thread#22
lavturo wants to merge 2 commits intophp:masterfrom
lavturo:fix_issue21

Conversation

@lavturo
Copy link
Contributor

@lavturo lavturo commented Apr 26, 2019

There was a part where there seemed to be a type mismatch and it was somehow working previously. However, the change to remove the deprecated method seemed to make the error pop up. Commented it out for the time being and tool still seems to work.

@lavturo
Copy link
Contributor Author

lavturo commented Apr 26, 2019

@weltling @hollyhuiLi Could you also check this?

Note: I think I know why my commits are overlapping now. Sorry about that. It shouldn't be a problem after this one.

@weltling
Copy link

@lavturo if you rebase the branch to master and force push, it should clean the history. Probably the branch wasn't forked on master, that's the reason the commit sha don't match.

Thanks.

@weltling
Copy link

@lavturo thanks for touching this part. My only concern while keeping the handling was, that otherwise a process can't be interrupted. Can it still be interrupted, when this exception is removed?

The reason for removing the deprecated method is because the implementation was based on Java 6 and in Java 8 it has issues or AFAIR it even refused to compile. Perhaps it was a mistake on my side, but i'm not sure one can use this deprecated functionality in Java 8. It's probably not future oriented to rely on that deprecated functionality, evermore it'll be painful after the Java 8 EOL :)

Thanks.

@lavturo
Copy link
Contributor Author

lavturo commented Apr 27, 2019

@weltling Reading the java doc: https://docs.oracle.com/javase/7/docs/technotes/guides/concurrency/threadPrimitiveDeprecation.html

If we wanted to stop a thread that waits a long time (if we want to timeout a test) then we would just use Thread.currentThread().interrupt(). So removing the Thread.stop() should be fine.

So far, when I tested it across multiple the runs, the UnsupportedOperationException doesn't show up anymore and it seems to run just fine.

@weltling
Copy link

@lavturo it's good for now as a fix to the general issues, thus merged. Still it might need to be revisited, as some tests might block the pipeline if running for too long.

Thanks.

@weltling weltling closed this Apr 29, 2019
@lavturo lavturo deleted the fix_issue21 branch October 17, 2019 17:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants