https://seongonion.tistory.com/144
위 수정사항을 Java Mail API에 PR로 올린지 일주일 후 답변이 왔다..!
하나는 내가 PR로 올린 코드에 대한 리뷰였고, 두 번째는 아마도 자신의 팀장..? 혹은 사수급으로 보이는 메인테이너에게 이슈 및 PR에 대한 설명 및 자신의 의견이었다.
코드 리뷰 먼저 살펴보자.
첫 번째는 기존의 master
라는 명명대신 root
라는 명명으로 변경해달라는 것이었다.
최근의 흐름에 맞는 네이밍이다. (실제로 깃헙에서도 3년 전부터 기본 브랜치를 master
에서 main
으로 이름을 변경했다)
바로 변경하고 따봉을 눌러줬다.
두 번째는 SharedFileInputStream
의 close()
메서드를 호출할 때 rootInputStream
에 대한 참조 역시 null
로 초기화해달라는 것이었다.
지금 보면 요구사항이 명확해보이는 리뷰이지만, 당시엔 마지막 말인 "but not closed" 에 꽂혀서 newStream()
으로 생성된 하위 스트림이 닫히더라도 루트 스트림의 파일이 "닫히지 않게" 해달라는 것으로 알아들었다.
그래서 "하위 스트림이 close()
를 호출하더라도 실제 네이티브 파일을 닫진 않는다.
그건 이미 SharedFile
의 close()
메서드에서 참조 카운트를 통해 처리해주고 있다" 라고 무려 프로젝트 메인테이너에게 약간은 길게길게 설명했다.. ㅎㅎ
Correct 하단다. 자기 말은 그냥 close()
호출할 때 다른 참조값들을 null
로 처리하듯 rootInputStream
도 null
로 처리해달라는 말이었다.
살짝 민망스해서 코멘트 남기고 수정했다. 따봉도 잊지 않았다.
다음으론 관련 테스트 코드를 추가해달라는 요청을 받았다.
PR 올리기 전에 해당 문제를 issue 탭에 올리면서 문제를 재현할 수 있는 간단한 테스트 코드를 올렸는데, 해당 코드를 정리해서 올려달라는 것이었다.
여기서 살짝 난관에 부딪혔다.
해당 프로젝트가 난생 처음 써보는 maven으로 빌드되고 있었고, 그간 익숙했던 junit5가 아닌 junit4로 테스트 환경이 구성되어있었다.
테스트 프레임워크야 뭐 junit4든 5든 레퍼런스를 참고할 게 많아서 문제 없었지만, 왜인지 테스트코드 쪽이 빌드가 되질 않았다..
정확히 말하면 빌드는 됐지만, 막상 테스트 코드 쪽 클래스를 열면 온갖 빨간 줄이 떴다. ㅠ
IDE의 도움을 받으니 module-info.java
파일을 수정해줘야할 것 같았는데, 아무리 찾아봐도 도저히 해결방법을 알 수가 없었다.
불행 중 다행인 것은, mvn test
커맨드를 통해 테스트를 실행하면 테스트 결과는 성공적으로 출력됐다.. ㅎㅎ
빨간 줄이 뜨더라도 우선 테스트 코드를 작성하고, 매번 커맨드로 테스트를 실행시켜서 성공 / 실패 여부를 확인했다.
다행히도 내 패치 후에는 기존 버전에서 실패했던 테스트 코드가 모두 성공했다!
코드 리뷰 뿐 아니라, 다른 메인테이너인 lukasj 라는 사람에게도 코멘트를 통해 해당 이슈와 PR에 대한 jmehrens 본인의 의견을 남겼다.
대략적으로 변역해보면, 진짜 문제는 SharedFileInputStream
과 SharedFile
모두에 finalizer가 있다는 것이었다.
finalizer의 메커니즘은 그것의 예측 불가능성과 퍼포먼스 이슈 등등의 문제로 JAVA 9부터 Deprecated 되었고, 이를 Cleaner API가 대체하고 있다. (Deprecated 배경에 대한 자세한 내용은 별도로 포스팅 해봐야겠다)
jmehrens 역시 두 클래스 모두에서 finalizer를 없애고 Cleaner API를 사용하는 것을 고려하는 것처럼 보인다.
물론 그렇게 되면 내가 PR로 제안한 방식은 적용이 필요없게 된다. 하핫 🥲
아래에서 계속 됩니다.
https://seongonion.tistory.com/146
'개발자취' 카테고리의 다른 글
검증 그 이상의 테스트 코드 (1) | 2023.12.26 |
---|---|
생에 첫 오픈소스 컨트리뷰트 도전기 (2) (0) | 2023.08.25 |
Java Mail API의 SharedFileInputStream 오류(로 추정되는 것) 수정해보기 (1) | 2023.07.23 |
[Spring] 테스트 코드 성능 개선기 (속도, 메모리) (0) | 2023.05.18 |
[Spring] 스프링 테스트코드 메모리(OOM)이슈 해결 아닌 회피기 (0) | 2023.04.02 |