ray icon indicating copy to clipboard operation
ray copied to clipboard

[Core] fix resource leak when cancel actor in phase of creating

Open wumuzi520 opened this issue 2 years ago • 0 comments

Why are these changes needed?

It is quite easy to cause resource/process leak when cancel an actor which constructor is time-consuming.

Related issue number

Closes #27743

Checks

  • [ ] I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • [ ] I've run scripts/format.sh to lint the changes in this PR.
  • [ ] I've included any doc changes needed for https://docs.ray.io/en/master/.
  • [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • [ ] Unit tests
    • [ ] Release tests
    • [ ] This PR is not tested :(

wumuzi520 avatar Aug 10 '22 11:08 wumuzi520

Can you also add a cpp unit test ?

Sure, I will do it.

wumuzi520 avatar Aug 11 '22 23:08 wumuzi520

Will not cherry pick this into 2.0

scv119 avatar Aug 16 '22 21:08 scv119

Will not cherry pick this into 2.0

Okey!

We are outing for the past few days, and this issue has been delayed. There is indeed a little problem with this PR and I also need to add some c++ unit tests.

wumuzi520 avatar Aug 17 '22 06:08 wumuzi520

Lmk if this is ready for the final review?

rkooo567 avatar Aug 26 '22 17:08 rkooo567

Lmk if this is ready for the final review?

Thanks, I added some cpp unit tests, and I think it is ready for review again. :)

wumuzi520 avatar Aug 27 '22 16:08 wumuzi520

The failure of mac-apple-ray-c-plus-plus-and-java is unrelated. BTW, I found that all tests inmac-apple-ray-c-plus-plus-and-java are passed, but the result is still failed. It is very likely that some cases have a segment error when exiting, I will find relevant colleagues to have a look.

wumuzi520 avatar Aug 28 '22 02:08 wumuzi520

Hi, @rkooo567 do you have any suggestions for this PR? :)

wumuzi520 avatar Aug 29 '22 14:08 wumuzi520