Skip to content

fix: getNextStringId 전략 인자가 싱글턴 기본 전략을 오염시키는 문제 수정 - #298

Open
z3rotig4r wants to merge 1 commit into
eGovFramework:mainfrom
z3rotig4r:fix/idgnr-strategy-leak
Open

fix: getNextStringId 전략 인자가 싱글턴 기본 전략을 오염시키는 문제 수정#298
z3rotig4r wants to merge 1 commit into
eGovFramework:mainfrom
z3rotig4r:fix/idgnr-strategy-leak

Conversation

@z3rotig4r

Copy link
Copy Markdown
Contributor

문제

AbstractIdGnrServicegetNextStringId(EgovIdGnrStrategy)getNextStringId(String)은 인자로 받은 전략을 인스턴스 필드 this.strategy에 대입한 뒤 무인자 getNextStringId()를 호출합니다. 대입된 전략은 호출이 끝나도 그대로 남습니다. ID 생성 서비스는 보통 싱글턴 빈으로 등록해 여러 업무가 함께 쓰므로, 한 곳에서 전략을 넘겨 한 번 호출하면 그 뒤로는 다른 곳의 무인자 getNextStringId()까지 그 전략으로 ID를 만듭니다.

기존 테스트에서도 이 오염이 드러납니다. 수정 전 코드로 EgovTableIdGnrServiceTest를 무작위 메서드 순서(MethodOrderer$Random, seed 1)로 실행하면 testIdGenStrategy가 실패합니다.

EgovTableIdGnrServiceTest.testIdGenStrategy:379 expected: <0> but was: <SMPL-####0>

앞서 실행된 testNotDefinedTableInfo가 싱글턴 서비스에 SMPL- 전략을 남긴 결과입니다. 이 테스트가 통과해 온 것은 메서드 실행 순서 덕분입니다.

필드 대입 자체도 문제입니다. 임의의 스레드가 런타임에 non-volatile 인스턴스 필드를 쓰는 동작이라, 동시 호출에서 데이터 레이스와 전략 교차 오염이 생깁니다.

수정

두 오버로드에서 필드 대입을 없애고 인자로 받은 전략으로 직접 ID를 만들도록 했습니다.

무인자 getNextStringId()final이고 본문이 strategy.makeId(getNextBigDecimalId().toString())입니다. 기존 코드를 펼치면 수정 후 코드와 글자 그대로 같은 식이고 getNextBigDecimalId() 호출도 1회로 같습니다. 시퀀스 소비량은 달라지지 않습니다.

수정 후에는 이 필드에 대한 런타임 쓰기가 사라지고 설정 시점의 setStrategy()만 남습니다.

검증

mvn -B -o -pl Foundation/org.egovframe.rte.fdl.idgnr test로 모듈 테스트 38건이 모두 통과합니다. 무작위 메서드 순서에서도 seed 1·2·3 모두 38건 전부 통과합니다.

인자로 넘긴 전략이 서비스 상태를 오염시키지 않는지 검증하는 테스트 IdGnrStrategyIsolationTest 3건을 추가했습니다. 전략 객체 인자 호출, 전략 빈 이름 인자 호출, setStrategy()로 지정한 전략이 무인자 호출에 계속 적용되는지를 각각 봅니다. 수정을 되돌리면 앞의 두 건이 실패합니다. 세 번째는 setStrategy 경로의 무회귀 가드라 수정 전후 모두 통과합니다.

영향 범위도 확인했습니다. strategy 필드는 private이고 읽는 곳은 getNextStringId()getStrategy()뿐입니다. 하위 클래스(AbstractDataIdGnrServiceAbstractDataBlockIdGnrServiceEgovSequenceIdGnrServiceImpl·EgovTableIdGnrServiceImpl)에는 이 필드를 참조하는 코드가 없습니다. EgovUUIdGnrServiceImpl은 이 클래스를 상속하지 않습니다. getNextStringId·getStrategy·setStrategy 참조도 idgnr 모듈 밖에는 없습니다.

동작 변경 안내

getNextStringId(전략)을 호출한 뒤의 무인자 getNextStringId()는 직전에 넘긴 전략을 더 이상 쓰지 않고 서비스에 설정된 기본 전략을 씁니다. 메서드 시그니처와 각 호출이 돌려주는 값은 그대로입니다. 바뀌는 것은 이전 호출이 이후 호출에 남기던 부작용뿐입니다.

전략을 계속 적용하려면 원래 그 용도인 setStrategy(EgovIdGnrStrategy)나 Spring 설정의 strategy 프로퍼티를 쓰면 됩니다. 이 경로가 그대로 동작한다는 것은 IdGnrStrategyIsolationTest#testSetStrategyContinuesToApplyToDefaultStringId가 검증합니다.

기존 동작에 기대는 코드가 실제로 있기는 어렵습니다. 공통컴포넌트의 getNextStringId 호출 181건 중 인자를 넘기는 곳은 테스트용 인터페이스 스텁 2개뿐이고 나머지는 전부 무인자 호출입니다.

getNextStringId(EgovIdGnrStrategy)와 getNextStringId(String)이 인자로 받은
전략을 인스턴스 필드 strategy에 대입해, 임시 전략으로 한 번 호출하면 이후
모든 호출자의 무인자 getNextStringId()가 그 전략을 사용했다. ID 생성 서비스는
싱글턴 빈이므로 호출자 간 상태가 새어나가고, 설정 주입용 setStrategy()가 별도로
있는데 조회 메서드가 상태를 바꾸는 것은 계약에도 어긋난다.

인자로 받은 전략을 지역 변수로만 사용하도록 바꿔 해당 호출에만 적용되게 했다.
public 시그니처와 기본 동작은 그대로다.

전략 격리와 setStrategy 무회귀를 검증하는 IdGnrStrategyIsolationTest를 추가했다.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant