CSE4006 · HANYANG UNIV · 2017 FALL

소프트웨어공학

설계 감각은 확실히 있는 사람이 만든 레포다. 다만 "테스트를 모두 통과했으므로 올바르게 작동한다"는 보고서의 핵심 주장이, 애초에 실행될 수 없는 테스트 위에 서 있다.

추적 파일279
Java 파일76
커밋131
기간2017.09–12
스택Java 8 / Ant
C종합
소견9 치명적11 중대5 경미합계 25

총평

과제 4개다. hw0(소셜 네트워크 그래프), VirutalWorld(에이전트 시뮬레이션), ParBST(fine-grained / read-write lock BST), LF_LL(lock-free sorted linked list). 커밋 131개는 2017-09-11부터 2017-12-27까지 실제로 학기 내내 흩어져 있고, 마감 직전 폭주하는 형태라 진짜 개발 이력이 맞다. 대리 작성이나 일괄 업로드의 흔적은 없다.

가장 잘한 것은 VirutalWorld다. 명세는 Fox/Rabbit/Gnat별로 AI 클래스를 만들라고 했는데, 이 사람은 "기억 → 전파(propagation) → 인식(behold) → 평가(judge/evaluate) → 결정(decide)"이라는 추상 파이프라인을 Actionable 하나에 올리고 동물별로는 judgegetForgetfulness만 오버라이드하게 만들었다. 학부 2학년 과제에서 보기 드문 추상화 수준이고, README에서 "AI 클래스는 명세 때문에 형식적으로 남겼다"고 스스로 밝힌 것도 정직하다.

가장 치명적인 것은 hw3다. 명세(ParBST/hw3.pdf)의 Part 2는 lock-free sorted linked listlock-free (leaf-oriented) binary search tree 두 개를 요구하고, 제출 리포지토리도 LF_LLLF_BST 두 개를 지정한다. 레포에는 LF_LL만 있다. git log --all --diff-filter=A로 전 이력을 훑어도 lock-free BST 파일이 생성된 커밋은 단 하나도 없다. 그런데 제출 보고서(ParBST/report.docx)와 README는 Part 2의 제목 자체를 "Lock-Free Sorted Linked List를 구현한다"로 고쳐 적어, 요구사항의 절반이 사라진 사실이 문서상 드러나지 않게 해 두었다.

그리고 검증층이 무너져 있다. 보고서는 "작성된 클래스는 테스트를 모두 통과했으므로 올바르게 작동한다고 볼 수 있습니다"를 Part 1과 Part 2에서 각각 반복하는데, RWBinaryTreeTest는 테스트 대상 트리를 생성하지 않는다(아래 참조). 남아 있는 실행 로그도 정확성 테스트가 아니라 성능 테스트 결과뿐이고, 그 숫자들은 서로 12,000배 모순된다. 동시성 자료구조 과제에서 "테스트가 통과했다"는 문장은 보고서의 유일한 정확성 근거인데, 그 근거가 근거가 아니다.

과제주제핵심 판정등급
hw0FriendGraph (소셜 네트워크)명세가 요구한 유일한 알고리즘(getDistance)이 BFS 레벨이 아니라 pop 순번을 거리로 반환한다. 확장 경로는 전부 예외로 끝난다.D+
hw2VirutalWorld (에이전트 시뮬레이션)설계는 이 레포에서 단연 최고. 다만 테스트가 0개이고, clone()이 자기 자신을 변형한다.B+
hw3-1ParBST (fine-grained / RW lock BST)없는 키를 지우면 NPE와 함께 트리 전역 락이 영구히 잠긴다. RW 트리는 테스트 대상이 null이라 한 번도 검증된 적이 없다.D+
hw3-2LF_LL (lock-free linked list)리스트 자체는 진짜 lock-free다. 그러나 과제의 나머지 절반(lock-free BST)이 없고, size()는 항상 0이다.D
검증 한계

이 머신에는 JDK가 설치되어 있지 않다. /usr/bin/javac는 존재하지만 실행하면 The operation couldn't be completed. Unable to locate a Java Runtime.을 낸다. 따라서 컴파일과 실행은 전부 미검증이며, 아래 모든 지적은 소스 정독과 레포에 커밋된 실행 로그 아티팩트에 근거한다. 실행이 필요한 주장에는 그 사실을 명시했다.

Homework 3 — 과제 범위

명세 PDF는 폰트 서브셋 인코딩이라 그대로는 읽히지 않아, 스트림을 풀고 치환 매핑(T→L, U→O, V→C, W→K, A→E, 2→R, 1→A, 3→T, P→I, Q→N, s→S, X→H, ?→D …)을 복원해 영문 토큰만 디코드했다. 아래 인용은 그 결과다.

치명적

과제의 절반인 lock-free BST가 아예 없다

hw3 Part 2는 lock-free 자료구조 두 개를 요구한다. 명세 본문은 "LOCK-FREE (SORTED) LINKED LIST를 구현하고, LOCK-FREE BINARY SEARCH TREE를 구현한다. LOCK-FREE BST는 LEAF-ORIENTED BST …"로 이어지고, 제출 지침은 리포지토리를 LF_LL (LINKED-LIST)LF_BST (BST) 두 개로 나누라고 지정한다.

레포 최상위에는 LF_LL만 있다. ParBST/src/collections/concurrent/ 아래에도 BinaryTree(fine-grained), RWBinaryTree(read-write lock), lockfree/LinkedList 셋뿐이고 lock-free 트리는 없다. 전체 브랜치·전체 이력에 대해 파일 추가 이력을 뒤져도 LF_BST나 leaf-oriented 구현이 등장하는 커밋은 없다. 다른 브랜치에 숨어 있는 것도 아니다 — 브랜치는 master 하나뿐이다.

더 나쁜 것은 문서 처리 방식이다. 명세의 Part 2 제목은 "Lock-Free Data Structure를 구현한다"인데, 제출 보고서와 README는 Part 2를 아래처럼 다시 제목 붙여 놓았다. 빠뜨린 요구사항을 "이건 원래 과제가 아니었다"로 만드는 문장이다. 같은 절에서 linked list의 기반 클래스를 두고는 "과제의 요구사항이 아니기 때문에 생략합니다"라고 요구사항 범위를 정확히 인지하고 있음을 보여주면서도, 실제로 빠진 BST에 대해서는 한 줄도 없다.

ParBST/README.md:49
## Part 2. Lock-Free Sorted Linked List를 구현한다.
// 명세 원문: "LOCK-FREE DATA STRUCTURE를 구현한다 ... LOCK-FREE (SORTED) LINKED LIST를
//            구현하고, LOCK-FREE BINARY SEARCH TREE를 구현한다"
// 제출 지침: REPOSITORY LF_LL (LINKED-LIST), LF_BST (BST)
경미

제출 보고서 표제에 과목 코드와 학교명이 모두 틀려 있다

ParBST/report.docxVirutalWorld/report.docx 두 편 모두 두 번째 줄이 CSE4065: 소프트웨어 공학 @ Hnaynag Univ.이다. 과목 코드는 CSE4006이고 학교명은 Hanyang이다. 한 번의 오타가 두 학기 분량의 제출물에 그대로 복사된 것이라, 제출 전 훑어보지 않았다는 뜻이다.

Homework 3 Part 1 — ParBST

hand-over-hand locking을 쓴 BinaryTree와, 직접 만든 ReentrantReadWriteOrderedLock 위에 올린 RWBinaryTree 두 구현이다. 커스텀 락을 처음부터 쓴 것은 학부 과제로서 야심찬 선택이고, 보고서에서 "Read Lock을 Write Lock으로 바로 업그레이드하면 Dead Lock이 발생" 같은 실제 겪은 문제를 서술한 대목은 진짜 삽질의 흔적이다. 문제는 그 서술이 코드와 맞지 않는다는 점이다.

치명적

없는 키를 지우면 NPE가 나면서 트리 전역 락이 영구히 잠긴다

delete()는 루트의 자식으로 내려갈 때 null 검사를 하지 않는다. 트리에 {5}만 있는 상태에서 delete(3)을 호출하면 cur = cur.leftnull이 되고 바로 cur.lock()에서 NullPointerException이 난다.

더 심각한 것은 예외가 나는 위치다. 전역 락 lock은 104행에서 잡히고 118행에서야 풀리는데, NPE는 그 사이인 117행에서 터진다. try/finally가 없으므로 전역 락도, 111행에서 잡은 루트 노드 락도 영원히 잠긴 채로 남는다. 이후 이 트리에 대한 모든 insert/delete/search는 첫 줄의 lock.lock()에서 무한 대기한다. 자료구조 하나가 통째로 벽돌이 된다.

같은 클래스의 insert()(76행)와 search()(245행)는 next == null을 제대로 검사한다. 즉 저자가 이 패턴을 모르는 게 아니라, delete의 루트 직하 분기 한 군데에서만 빠뜨린 것이다.

        lock.lock();                      // ← 전역 락 획득
        ...
            LockableNode cur = root;
            LockableNode par;
            cur.lock();

            int compare = cur.data.compareTo(data);
            if (compare != 0) {
                par = cur;
                cur = compare > 0 ? cur.left : cur.right;
                cur.lock();                   // ← cur이 null이면 NPE. 전역 락은 잡힌 채로.
                lock.unlock();                // ← 여기까지 도달하지 못한다
치명적

탐색이 실패하면 노드 락을 반납하지 않고 빠져나온다

delete()의 탐색 루프는 대상을 못 찾고 리프를 지나칠 때 return false로 빠져나가는데, 그 시점에 par가 잡고 있는 락을 풀지 않는다.

{5, 3}인 트리에서 delete(4)를 추적해 보면: 138행에서 노드 5의 락을 풀고 par = cur(노드 3, 이미 락 보유)로 옮긴 뒤, 143행에서 cur = cur.right = null이 되어 146행에서 return false한다. 노드 3의 락은 영구히 잠긴 상태로 남는다. 이후 노드 3을 지나가야 하는 모든 연산이 그 자리에서 멈춘다.

이 두 결함은 합쳐서 하나의 결론을 만든다 — 이 BST는 "찾지 못하는 삭제"에 전혀 견디지 못한다. 그리고 BinaryTreeTest의 어떤 테스트도 존재하지 않는 키를 삭제하지 않는다. delete 계열 테스트는 전부 방금 넣은 키만 지운다(48–62행).

                    } else {
                        par.unlock();
                        par = cur;                // ← cur의 락이 par로 인계됨

                        compare = cur.data.compareTo(data);
                        if (compare > 0) cur = cur.left;
                        else cur = cur.right;
                    }

                    if (cur == null) return false;   // ← par.unlock() 없음. 락 누수.
                    else cur.lock();
치명적

RWBinaryTreeTest는 트리를 생성하지 않는다 — RW 트리는 한 번도 검증된 적이 없다

보고서가 Part 1에서 가장 공들여 설명하는 것이 read-write lock 버전이다. 커스텀 OrderedLock을 새로 만든 이유, downgrade()를 추가한 이유, dead lock을 만난 경위까지 세 문단을 쓴다. 그런데 그 구현의 테스트 클래스는 @Before에서 pool만 만들고 tree는 만들지 않는다. 필드 선언(14행)만 있고 초기화가 없으므로 treenull이며, 6개 테스트 전부가 첫 줄에서 NullPointerException으로 죽는다.

삭제된 자리에 빈 줄 두 개가 그대로 남아 있는 것이 이 코드의 이력을 말해 준다 — 한때는 있었고, 지워졌고, 아무도 다시 돌려 보지 않았다.

그 상태에서 보고서는 이렇게 쓴다: "작성된 BinaryTree 클래스는 테스트를 모두 통과했으므로 올바르게 작동한다고 볼 수 있습니다." 문장은 BinaryTree만 지칭하지만, 문단 전체가 두 구현을 함께 설명하는 절 안에 있고 RWBinaryTree에 대해서는 검증 근거를 따로 대지 않는다. 레포에 남은 실행 로그(ParBST/results/)도 BinaryTreePerformanceTest의 성능 측정치뿐이고, 정확성 테스트 결과는 한 건도 보존되어 있지 않다.

public class RWBinaryTreeTest {
    private static RWBinaryTree<Integer> tree;   // ← 끝까지 대입되지 않는다
    private static concurrent.Pool pool;
    ...
    @Before
    public void makeInstance() throws Exception {
                                                 // ← tree = new RWBinaryTree<>(); 가 있어야 할 자리

        pool = new concurrent.Pool(4);
    }

    @Test
    public void insert() throws Exception {
        numbers.forEach((e) -> tree.insert(e));   // ← NPE
치명적

read → write 락 승격이 원자적이지 않다 — 보고서가 해결했다고 주장하는 바로 그 문제

보고서는 downgrade()를 만든 이유를 이렇게 설명한다: "WriteLock.unlock(), ReadLock.lock()과 같이 구현하게 되면 Write Lock을 해제하고 Read Lock을 잡기 전에 다른 스레드가 Write Lock을 잡아버릴 위험이 있기에 … 하향시키는 기능을 추가하였습니다." 내려오는 방향(write→read)은 실제로 downgrade()로 원자화했다.

그런데 올라가는 방향은 바로 그 방식 그대로다. write()는 read lock을 풀고 write lock을 잡는 두 문장으로 시작한다. 그 사이에 다른 스레드가 write lock을 잡고 노드를 바꿀 수 있다. 그래서 insert가 "next == null이니 여기에 새 노드를 달자"고 판단한 뒤 write() 안에서 다시 c.left != null을 재검사하게 되어 있는데(122행), 이는 저자도 이 틈을 알고 있었다는 뜻이다. 문제는 그 재검사가 실패했을 때다(아래).

그리고 그 사이에 놓인 두 줄이 더 있다. catch (RuntimeException e) {}는 임계 구역 안에서 발생한 모든 런타임 예외를 통째로 삼키고, 그 다음 88행의 return true실패한 쓰기를 성공으로 보고한다. 위의 BinaryTree에서 본 것과 똑같은 null 역참조가 여기서 일어나면, 호출자는 트리가 정상적으로 갱신되었다고 믿는다.

        boolean write(final Function<LockableNode, Boolean> f) {
            readLock.unlock();
            writeLock.lock();            // ← 이 두 줄 사이가 무방비. 승격이 원자적이지 않다.
            try {
                return f.apply(this);
            } catch (RuntimeException e) {  // ← 전부 삼킴
            } finally {
                writeLock.downgrade();
            }
            return true;                 // ← 예외가 났어도 "성공"
        }
중대

문서화된 재시도 계약을 호출자가 지키지 않는다

write()의 javadoc은 이렇게 명시한다: "@param f Return false raise dirty write when acquire write lock, must configure logic to re-try". 즉 false는 "경합이 났으니 다시 시도하라"는 신호다.

delete()의 호출부는 재시도하지 않는다. 170–186행의 par.write(...)false를 반환하면 if 블록을 그냥 통과해 193–196행으로 떨어지는데, 이 시점의 compare는 0이므로 compare > 0이 false가 되어 오른쪽 서브트리로 내려간다. 삭제하려던 키와 같은 값을 찾던 중이었으므로 오른쪽 서브트리에는 그 키가 있을 수 없고, 결국 루프는 리프까지 내려가 false를 반환한다. 경합이 한 번이라도 발생하면 삭제가 조용히 누락된다.

중대

RWBinaryTree.delete()가 없는 키에 대해 true를 반환한다

루트의 해당 방향 자식이 없으면(즉 키가 트리에 없으면) 157–161행이 락을 풀고 return true한다. 트리가 아예 비어 있을 때도 146–148행에서 lock.unlock()만 하고 함수 끝의 return true(219행)로 떨어진다. 반면 더 깊은 곳에서 실패하면 199행에서 false를 반환한다. 같은 함수 안에서 "찾지 못함"의 반환값이 위치에 따라 달라진다.

대응하는 BinaryTree는 같은 자리에서 정확히 false를 반환한다(107행). 즉 이식 과정에서 뒤집힌 것이고, 테스트가 delete반환값을 한 번도 단언하지 않기 때문에(assertFalse(tree.search(e))만 본다) 잡히지 않았다.

중대

inOrderHelper는 어떤 노드의 락도 풀지 않는다

BinaryTree.inOrderTraversal(286–307행)은 루트를 잠그고 inOrderHelper를 호출한다. 헬퍼는 좌·우 자식을 잠그며 재귀하지만 unlock() 호출이 한 줄도 없다. 순회 한 번이면 트리 전체 노드가 잠긴 채로 남는다. ReentrantLock이라 같은 스레드는 재진입할 수 있지만, 다른 스레드는 이후 어떤 연산도 수행할 수 없다.

바로 위의 preOrderHelper(270–283행)는 f.accept(node) 직후 node.unlock()을 호출한다. 같은 파일에서 한쪽은 맞고 한쪽은 빠진 것이라, 복사 후 수정 과정의 누락으로 보인다. RWBinaryTree의 같은 메서드(340–353행)에는 node.unlock()이 들어 있다 — 즉 저자가 나중에 고쳤으나 BinaryTree 쪽에는 반영하지 않았다.

경미

"Reentrant"라는 이름과 달리 재진입이 불가능하다

ReentrantReadWriteOrderedLock은 hold count를 관리하지 않는다. lock()entries.put(threadId, entry)로 기존 항목을 덮어쓰고(62행) waiter에 항목을 하나 더 넣는다(63행). 같은 스레드가 write lock을 이미 쥔 상태에서 다시 lock()을 호출하면 validate()는 큐 머리의 항목이 자기 것과 equals이므로 true를 반환하고, 64–67행의 do { while (!validate()) writer.await(); } while (writing);writing == true이므로 await() 없이 CPU를 태우며 무한 반복한다. 자기 잠금이 아니라 자기 스핀이다.

또한 downgrade()(150–161행)는 writing = false로 바꾸면서 어떤 condition에도 signal을 보내지 않아, writer.await()에 잠든 스레드가 깨어날 기회를 잃는다.

잘한 것

커밋 "Fix RWBinaryTree hold read lock problem"(2017-11-26), "Fix signal all to waiting"(2017-11-28), "Fix search function invalid"(2017-11-21)은 자기 코드의 동시성 버그를 스스로 찾아 고친 기록이다. 락 프로토콜 문제를 디버깅해 본 사람만 쓸 수 있는 커밋 메시지이고, 보고서에서 "Read Lock을 Write Lock으로 바로 업그레이드할 수 없기 때문에 Dead Lock이 발생"이라고 원인을 특정한 대목도 실제 관찰의 산물이다. 문제는 그 다음이다 — 발견은 했는데, 그 발견을 잡아 줄 테스트를 남기지 않았다.

Homework 3 Part 2 — LF_LL

먼저 이름값에 대한 판정부터 하자면 — 이건 진짜 lock-free다. synchronizedLock을 lock-free라고 부르는 흔한 사기가 아니다. AtomicMarkableReference로 노드의 next 포인터와 논리 삭제 마크를 한 워드에 묶었고(30행), find()는 마크된 노드를 지나가며 CAS로 물리적으로 잘라내고(71행), add/remove는 CAS 실패 시 find부터 재시도한다. Herlihy & Shavit의 lock-free list를 제대로 이해하고 옮긴 코드다. 메모리 가시성도 AtomicMarkableReference가 책임지므로 별도 volatile이 필요 없고, 실제로 빠져 있지 않다. 문제는 옮기면서 달라진 세 군데다.

중대

attemptMark는 마크를 검사하지 않는다 — 두 스레드가 같은 노드 삭제에 동시에 "성공"한다

원본 알고리즘의 논리 삭제는 cur.next.compareAndSet(suc, suc, false, true)다. 여기서 기대 마크 false가 핵심이다. 이미 마크된 노드에 대해서는 CAS가 실패하므로 정확히 한 스레드만 삭제에 성공하고 true를 반환한다.

이 코드는 attemptMark(suc, true)를 쓴다. AtomicMarkableReference.attemptMark는 레퍼런스만 비교하고 현재 마크는 보지 않는다. 따라서 스레드 A가 이미 마크한 노드에 대해 스레드 B가 attemptMark를 호출하면 (마크를 true에서 true로 쓰는 것이므로) 성공하고, B도 return true한다. remove()는 "내가 이 원소를 제거했다"를 뜻해야 하는데, 같은 원소에 대해 두 스레드가 모두 참을 받는다.

이것이 잡히지 않은 이유는 명확하다. LinkedListTestremove의 반환값을 한 번도 확인하지 않는다(44행: pool.push(() -> list.remove((Integer) e))). 그리고 각 키를 정확히 한 번씩만 지운다.

            else {
                Node<T> suc = cur.next.getReference();
                snip = cur.next.attemptMark(suc, true);   // ← 현재 마크를 검사하지 않는다
                if (!snip) continue;                     // 원본: compareAndSet(suc, suc, false, true)
                pre.next.compareAndSet(cur, suc, false, false);
                return true;
            }
중대

size()는 무슨 일이 있어도 0을 반환한다

counter 필드는 파일 전체에서 세 번 등장한다 — 선언(20행), 생성자에서 counter = 0(92행), 그리고 size()return counter(181행). addremove 어디에서도 증감하지 않는다. 100만 개를 넣어도 size()는 0이다.

죽은 필드 자체보다 중요한 것은 이것이 테스트가 전혀 확인하지 않는 축이라는 점이다. 자료구조의 원소 개수는 동시 삽입/삭제의 정확성을 검증하는 가장 값싼 불변식인데, 구현도 검증도 없다. 마침 위의 attemptMark 결함이 정확히 이 축에서 드러났을 결함이다.

    private int counter;          // :20  선언
        counter = 0;              // :92  생성자. 이후 아무도 건드리지 않는다.
    @Override
    public int size() {
        return counter;            // :181 항상 0
    }
중대

정렬 기준이 compareTo가 아니라 hashCode

클래스는 T extends Comparable<? super T>를 요구하지만 compareTo한 번도 호출하지 않는다. 키는 전부 item.hashCode()다(35, 102, 125, 165행). 보고서는 이를 "자바는 hashCode를 지원함으로 이를 통해 구현하였습니다"라고 포인터의 대체물로 설명하는데, 명세가 요구한 것은 sorted linked list다. hashCodecompareTo와 일관될 의무가 없으므로 정렬 순서가 자료형의 순서와 무관해진다.

더불어 해시 충돌이 곧 정확성 오류가 된다. 서로 다른 두 원소의 hashCode가 같으면, 뒤에 넣은 원소는 "이미 존재함"으로 false를 받고(108행) 저장되지 않으며, 넣은 적 없는 원소에 대해 containstrue를 반환한다.

테스트가 이를 못 잡는 이유도 명확하다 — 테스트 원소가 전부 Integer이고 Integer.hashCode()는 값 자신이라, 이 구현에서 유일하게 버그가 드러나지 않는 자료형이다.

중대

성능 로그의 숫자가 자기 자신과 12,000배 모순되는데, 보고서는 그 숫자를 해석한다

레포에 커밋된 LF_LL/results/Test Results - LinkedListPerformanceTest_MAC.html을 그대로 읽으면 이렇다. 같은 testB 안에서 10만 건 삽입이 303,515 ms(5분)인데, 그 직후 같은 10만 건의 insert+search 혼합이 25 ms / 14 ms / 111 ms다. 리스트에 이미 10만 노드가 있고 contains가 O(n) 선형 탐색인데 10만 번의 연산이 25 ms에 끝나는 것은 물리적으로 불가능하다. 두 구간 중 하나는 실제로 일하지 않았다는 뜻이다.

구조를 보면 원인 후보가 분명하다. 두 구간의 차이는 pool이 새로 만들어지느냐다(LinkedListPerformanceTest.java:86에서 재생성). 그 앞 구간(72–75행)은 @Before에서 만들어 testA와 공유하는 풀을 쓰는데, concurrent.Pool.join()은 워커 스레드를 종료시키므로 한 번 join한 풀은 재사용할 수 없다. 다만 이 머신에 JDK가 없어 실행으로 확정하지는 못했다 — 추정으로 남긴다.

확정적으로 말할 수 있는 것은 이것이다. 보고서 Figure 4–5는 이 숫자들을 근거로 "Lock Free 구조체의 특성상 다수의 스레드가 실행해도 Lock에 의한 대기가 발생하지 않기 때문", "이론적인 성능 향상의 최대값이 존재하는 것처럼 보여집니다" 같은 결론을 이끌어낸다. 자기 측정치가 자기 측정치와 12,000배 어긋난다는 사실을 확인하는 문장은 한 줄도 없다. 측정 설계에서 가장 먼저 해야 할 일 — 숫자가 말이 되는지 보기 — 이 생략되었다.

Inserting 100000 numbers takes 303515ms
Insert and Search ratio 1:1, 100000 numbers takes 25ms
Insert and Search ratio 1:4, 100000 numbers takes 14ms
Insert and Search ratio 1:9, 100000 numbers takes 111ms
// 같은 리스트, 같은 연산 수. contains는 O(n).
경미

contains()의 죽은 변수와 센티널 충돌

169행의 Node suc = cur.next.get(marked);에서 suc는 대입만 되고 쓰이지 않는다(부수효과인 marked 채우기만 노린 것이지만, 원본 알고리즘에서는 이 호출이 루프 에 있다). 또한 raw type Node를 쓰고 있어 제네릭 경고가 난다.

더해서, item.hashCode()Integer.MIN_VALUE인 원소에 대해서는 while (cur.key < key) 루프가 한 번도 돌지 않아 cur이 헤드 센티널인 채로 남고, 헤드의 키가 정확히 Integer.MIN_VALUE(87행)이므로 넣은 적 없는 원소에 true를 반환한다. 대칭적으로 hashCodeInteger.MAX_VALUE인 원소는 테일 센티널(89행)과 충돌해 add가 언제나 false를 반환하며 절대 저장되지 않는다.

공통 인프라 — concurrent.Pool

두 hw3 모듈이 동일하게 쓰는 직접 구현 스레드 풀이다(LF_LLParBST의 파일이 바이트 단위로 같다). hw3의 모든 동시성 테스트와 모든 성능 측정이 이 클래스 위에서 돌았으므로, 여기의 결함은 위의 모든 수치에 전가된다.

치명적

워커 스레드에서 터진 AssertionError는 아무 데도 도달하지 않는다 — 테스트가 실패할 수 없다

LinkedListTest.contains(53행)와 BinaryTreeTest.searchParallel(75행)은 JUnit 단언을 워커 스레드 안에서 실행한다. assertTrue가 실패하면 AssertionError가 던져지는데, 이것은 Error이지 RuntimeException이 아니다. Pool.Worker.run()의 catch 절은 RuntimeException만 잡으므로(67행) AssertionError는 그대로 run() 밖으로 나가 그 워커 스레드를 조용히 죽인다.

JUnit은 테스트 메서드를 실행한 스레드에서만 예외를 수집한다. 따라서 실패는 메인 스레드에 전달되지 않고, pool.join()은 이미 죽은 스레드를 join하며 즉시 반환하고, 테스트는 초록색으로 통과한다. 구현이 무엇을 반환하든 상관없이 통과한다.

보고서의 핵심 주장 — "테스트를 모두 통과했으므로 올바르게 작동한다고 볼 수 있습니다" — 가 이 지점에서 무너진다. 통과는 정확성의 증거가 아니라, 실패를 관측할 경로가 없다는 증거다.

LF_LL/src/concurrent/Pool.java:65–69 · LF_LL/test/.../LinkedListTest.java:53
// Pool.java:65-69
                try {
                    task.run();
                } catch (RuntimeException e) {   // ← AssertionError는 Error. 안 잡힌다.
                    e.printStackTrace();
                }

// LinkedListTest.java:53
        numbers.forEach((e) -> pool.push(() -> assertTrue(list.contains(e))));
중대

join()이 대기 중인 워커를 깨우지 않고, terminatevolatile이 아니다

join()terminate = true로 바꾸고 곧바로 각 워커를 join한다. 그런데 queue.notify()notifyAll()을 호출하지 않는다. 큐가 빈 상태에서 queue.wait()에 들어가 있던 워커는 깨울 사람이 없으므로 영원히 잠들어 있고, join()은 영원히 반환하지 않는다. 종료 시점에 워커가 모두 바쁘게 돌고 있었다면 우연히 빠져나오지만, 이는 경쟁 조건에 기댄 것이다.

게다가 terminatevolatile이 아니고(7행), 쓰기는 join()에서 동기화 블록 밖에서 일어난다(30행). 읽기(55행)만 synchronized (queue) 안에 있으므로 happens-before가 성립하지 않는다. 워커가 갱신을 영영 보지 못할 수 있다. 이 저자는 LF_LL에서는 AtomicMarkableReference로 가시성을 정확히 다루면서, 정작 그 코드를 구동하는 풀에서는 같은 문제를 놓쳤다.

덧붙여 push()notify()(한 스레드만 깨움)를 쓰고, 큐 자체는 이미 스레드 안전한 LinkedBlockingQueue다(9행). take()/put()을 쓰면 synchronized/wait/notify가 전부 필요 없어지고, 애초에 java.util.concurrent.ExecutorService로 대체 가능하다.

LF_LL/src/concurrent/Pool.java:7, 29–37, 52–57
    private boolean terminate;              // :7  volatile 아님

    public void join() {
        terminate = true;                   // :30 synchronized 밖에서 쓰기
        for (Worker worker : workers)
            try { worker.join(); }          // ← notifyAll() 없음
            ...

                synchronized (queue) {      // :52
                    while (queue.isEmpty()) {
                        if (queue.isEmpty() && terminate) return;
                        queue.wait();           // :57 깨워 줄 사람이 없다

Homework 1 — hw0 (FriendGraph)

명세(hw0/hw0.pdf)는 단 하나를 요구한다 — 무방향 그래프로 소셜 네트워크를 모델링하고 두 사람 사이의 거리를 계산하라. 인접 리스트를 int[][]로 직접 관리하고 BFS용 원형 큐까지 손수 만든, 몸풀기 과제치고는 성실한 구조다. 그런데 정작 요구한 그 하나가 틀렸다.

치명적

getDistance는 BFS 레벨이 아니라 "몇 번째로 꺼냈는가"를 거리로 반환한다

last는 큐에서 노드를 하나 꺼낼 때마다 1씩 증가한다(268행). 이것은 BFS의 레벨이 아니라 지금까지 방문 처리한 노드 수다. 두 값이 일치하는 것은 각 레벨에 노드가 정확히 하나뿐인 경우 — 즉 그래프가 일직선일 때뿐이다.

다섯 명으로 반례가 만들어진다. John, Merry, Mike, Steve, Kate를 넣고 addFriendship을 John–Mike, John–Merry, Merry–Steve, Mike–Kate 순으로 호출하면 John의 인접 리스트는 [Mike, Merry]가 된다. 추적하면: John을 꺼내며 Mike와 Merry를 거리 1로 기록(last→2), Mike를 꺼내며 Kate를 거리 2로 기록(last→3), Merry를 꺼내며 Steve를 visit[Steve] = last = 3으로 기록하고 반환한다. 정답은 2인데 3을 반환한다. (JDK가 없어 실행 확인은 못 했고, 소스 추적으로 유도한 결과다.)

테스트가 이를 놓친 이유는 그래프가 너무 작기 때문이다. FriendGraphTest.getDistance(86–99행)는 네 명에 간선 세 개로, 한 레벨에 여러 노드가 있어도 목표가 두 번째 pop에서 바로 발견되는 배치라 우연히 2가 나온다. 반례를 만들려면 목표와 무관한 노드가 하나 더 pop되기만 하면 된다.

hw0/src/cse4006/FriendGraph.java:246, 256–269
        int last = 1;                                    // :246
        ...
        while (!q.isEmpty()) {
            int v = q.pop();

            for (int k = 0; k < network[v][0]; k++) {
                if (visit[network[v][k + 1]] == -1) {
                    visit[network[v][k + 1]] = last;      // ← 거리가 아니라 pop 순번
                    if (network[v][k + 1] == j) {
                        return visit[network[v][k + 1]];
                    }
                    q.add(network[v][k + 1]);
                }
            }
            last += 1;                                // :268 레벨이 아니라 pop마다 증가
        }
치명적

17번째 사람을 추가하면 ArrayIndexOutOfBoundsException이 난다

count-1에서 시작하고 persons의 길이는 size(기본 16)다. 유효한 인덱스는 0–15인데, 확장 조건이 count > size다. 17번째 호출에서 count가 16이 되면 16 > 16은 거짓이므로 확장이 일어나지 않고 persons[16]에 그대로 쓴다. >=여야 한다.

비교 연산자를 고쳐도 두 번째 결함이 남는다. adjust()의 확장 분기(71–79행)는 기존 행을 복사하기만 하고 새로 생긴 행을 초기화하지 않는다. latest[16..31]null이 되고, 그 인덱스의 사람에게 친구를 추가하는 순간 connection[0]에서 NullPointerException이 난다. 반면 최초 생성 분기(66–70행)는 newConnection()으로 각 행을 제대로 만든다 — 즉 초기화 코드를 알면서 확장 경로에만 빠뜨렸다.

hw0/src/cse4006/FriendGraph.java:177–187
    public void addPerson(Person person) {
        if (isPerson(person))
            return;

        count += 1;
        if (count > size) {        // ← count == size일 때 확장되지 않는다
            expand();
        }

        persons[count] = person;  // ← persons.length == size. count == size면 예외.
    }
치명적

인접 리스트 "확장"이 배열을 오히려 줄인다

newConnection(size, connection)은 새 배열의 길이를 Math.min(connection[0], size + 1)로 잡는다. connection[0]현재 저장된 원소 수이지 용량이 아니다. 호출 조건(isFullConnection, 148행)이 바로 connection[0] == connection.length - 1이므로, 길이 17짜리 꽉 찬 배열에 대해 새 배열의 길이는 min(16, 35) = 16원본보다 작다.

그 다음 줄이 array[0] = Math.min(connection[0], size) = 16이고, 복사 루프가 i < 16 범위에서 array[i + 1]에 쓰므로 마지막 반복이 길이 16 배열의 array[16]에 접근한다. ArrayIndexOutOfBoundsException: 16이다. 한 사람의 친구가 17명이 되는 순간 addFriendship이 터진다.

FriendGraphTest의 어떤 케이스도 4명 이상의 그래프나 초기 용량을 넘는 친구 수를 만들지 않는다. 성장 경로 전체가 테스트되지 않은 채 남아 있고, 그 전체가 깨져 있다.

hw0/src/cse4006/FriendGraph.java:49–56 · 호출부 :197–199
    private final int[] newConnection(final int size, final int[] connection) {
        int array[] = new int[Math.min(connection[0], size + 1)];  // ← connection[0]은 용량이 아니라 개수
        array[0] = Math.min(connection[0], size);
        for (int i = 0; i < array[0]; i++) {
            array[i + 1] = connection[i + 1];                    // ← 마지막 반복에서 범위 초과
        }
        return array;
    }
// :197  if (isFullConnection(connection))                        // connection[0] == length-1
// :198      connection = newConnection(connection.length * 2, connection);
중대

원형 큐가 pop마다 배열을 재할당해 BFS를 O(V²)로 만든다

Queue.pop()은 원소를 꺼내기 전에 getSize() < cap / 2adjust(cap / 2)를 호출한다. adjust는 큐를 전부 비워 새 배열로 옮기는 O(n) 연산이다. BFS처럼 큐 길이가 용량의 절반 이하로 오래 머무는 사용 패턴에서는 거의 모든 pop이 전체 복사를 유발한다. 게다가 용량이 절반씩 줄어들다 다시 add에서 두 배로 늘어나는 진동까지 겹친다.

결과적으로 getDistance의 계산 복잡도가 O(V+E)가 아니라 O(V²)가 된다. 축소 조건은 보통 size < cap / 4 정도로 잡아 amortized O(1)을 지킨다. QueueTest.popTest(51–62행)는 원소 5개로 정확히 이 축소 사슬을 타고도 통과하는데, 단언이 값만 보고 재할당 횟수는 보지 않기 때문이다.

    public T pop() {
        if (isEmpty()) {
            return null;
        } else {
            if (getSize() < cap / 2) {
                adjust(cap / 2);      // ← O(n) 전체 복사. pop마다 발생한다.
            }
            Object element = elements[front];
경미

equals만 있고 hashCode가 없으며, 그나마 쓰이지 않는다

Person.equals(46–60행)는 Class.isAssignableFrom까지 챙긴 꼼꼼한 구현인데 hashCode를 함께 재정의하지 않았다. equals/hashCode 계약 위반이라 HashSet·HashMap에 넣는 순간 깨진다. 그리고 정작 FriendGraphpersons[i].getName().equals(name)으로 문자열을 직접 비교하므로(109, 157행) 이 equals를 한 번도 호출하지 않는다.

덧붙여 Queue의 생성자는 this.cap = cap + 1인데 배열은 new Object[cap]으로 만들어(15–18행) capelements.length가 1만큼 어긋난다. 첫 adjust() 이후로는 cap = size로 정정되므로 일관성이 그때부터 바뀐다. 실제 폭발까지 이어지지는 않지만, 불변식이 시점에 따라 달라지는 상태다.

Homework 2 — VirutalWorld

Rabbit/Fox/Gnat과 각각의 AI를 구현하고, 자기가 상상한 동물을 하나 추가하라는 과제다. 이 레포에서 가장 잘 만들어진 모듈이고, 판정도 여기만 다르다. 다만 소프트웨어공학 과목의 과제로서 테스트 파일이 0개라는 점은 그대로 감점 사유다(hw0에는 4개, hw3에는 9개가 있다).

중대

clone()이 복제가 아니라 자기 자신을 변형한다

Object.clone()의 계약은 "이 객체의 복사본을 만든다"이지 "이 객체를 바꾼다"가 아니다. 그런데 이 구현은 첫 줄에서 수신자의 energy를 절반으로 깎는다(442행). 번식 시 부모가 에너지를 나눠 준다는 게임 규칙 자체는 타당하지만, 그 규칙을 clone()에 넣으면 "복제했을 뿐인데 원본이 변했다"가 된다. 이 메서드를 디버깅·로깅·상태 스냅샷 목적으로 호출하는 순간 시뮬레이션 상태가 조용히 망가진다. breed() 안에 놓았어야 할 로직이다.

또한 super.clone()은 얕은 복사라 edible HashSet(32행), world, ai, nowLoc, preLoc이 부모·자식 간에 참조로 공유된다. memoryinfoinitialized = false와 새 Information으로 분리 처리했으니, 나머지를 빠뜨린 것은 의도가 아니라 누락으로 보인다. 그리고 실패 시 null을 반환하는데(453행), 호출부인 breed()는 그 nullworld.add()에 그대로 넘기고 발생한 예외를 catch (Exception)으로 삼킨다(421–428행).

    @Override
    public Actionable clone() {
        energy = (int) floor(energy / 2);   // ← this를 변형한다. clone의 계약 위반.
        final Actionable clone;
        try {
            clone = (Actionable) super.clone();  // 얕은 복사: edible/world/ai 공유
            clone.energy = this.energy;
            clone.initialized = false;
            clone.info = new Information(info.getGeneration());
            return clone;
        } catch (Exception e) {
            e.printStackTrace();
        }
        return null;                        // ← 호출부에서 world.add(null, ...)
    }
중대

"skeleton을 변경하지 않았다"는 보고서 주장과 실제 커밋이 다르다

보고서와 README는 첫 문단에서 "주어진 skeleton의 구조를 변경하지 않으면서 높은 재사용성과 캡슐화를 위한 디자인과 설계를 진행해야 한다"고 명세를 요약한다. 그러나 커밋 43e2cbc "Fix implementation"은 skeleton의 Direction enum에서 필드 타입과 공개 메서드의 반환 타입을 바꾼다(Pair<Integer, Integer>Location). 커밋 b4378ab "Fix"는 skeleton의 World 인터페이스에 getGeneration(), getCount(), getCount(Actors) 세 메서드를 추가한다.

인터페이스에 메서드를 추가하는 것은 논쟁의 여지가 있지만, getValue()의 반환 타입을 바꾸는 것은 프레임워크 API의 파괴적 변경이다. 최소한 보고서에 "이런 이유로 skeleton의 이 부분을 수정했다"는 한 줄이 있어야 했다. 흥미롭게도 World.java에는 저자가 직접 붙인 @Custom Improve 태그가 있어 스스로 변경 사실을 표시해 두었는데, 그 표시가 보고서까지 올라오지는 않았다.

VirutalWorld/src/faceduck/skeleton/util/Direction.java:16–24 (커밋 43e2cbc)
-    Pair<Integer, Integer> position;
+    Location position;

-    public Pair<Integer, Integer> getValue() {
+    public Location getValue() {      // ← skeleton 공개 API의 반환 타입 변경
경미

절대 실행될 수 없는 finalize()

"죽을 때 world에서 제거한다"는 주석과 함께 finalize()를 재정의했다(59–67행). 그런데 finalize()는 GC가 객체를 수거할 때 호출되고, world가 액터를 참조하고 있는 한 그 객체는 수거되지 않는다. 즉 주석이 말하는 상황에서는 절대 호출되지 않는다. 실제 제거는 act()가 에너지 소진 시 world.remove(this)를 직접 호출해 처리하고 있으므로(133행) 이 메서드는 순수한 죽은 코드다. 게다가 finalize()를 재정의하면 해당 클래스의 모든 인스턴스가 GC에서 2패스 처리를 받아 회수가 지연된다 — 수만 개 액터가 생멸하는 시뮬레이션에서는 실측 가능한 비용이다.

잘한 것

명세는 FoxAI, RabbitAI, GnatAI를 각각 구현하라고 했다. 이 사람은 동물마다 AI 로직을 복붙하는 대신, "기억을 전파하고(propagation) 시야를 갱신하고(behold) 대상을 점수화하고(judge) 위치를 평가해(evaluate) 행동을 고르는(decide)" 파이프라인을 Actionable에 한 번만 구현하고, 동물별로는 judgegetForgetfulness만 오버라이드하게 만들었다. getCoolDown() / 2만큼 propagation을 반복해 "쿨다운 동안 남들도 움직였을 것"을 모델링한 부분(Actionable.java 113행)은 과제가 요구하지 않은 수준의 설계다. 그 결과 AI 클래스들은 actionable.decide() 한 줄로 줄었고, 저자는 README에서 "명세에 각 AI 클래스를 구현하라는 조건이 있었으므로 형식적으로 남겨 두었다"고 그 사실을 감추지 않고 밝힌다. 코드에도 // @TODO anti pattern, It's cleaner not to use AI(119행)라고 스스로 적어 두었다. 이 레포 전체에서 문서와 코드가 가장 정확하게 일치하는 대목이다.

레포 위생

추적 파일 279개 중 .class 파일이 114개다. VirutalWorld/classes/VirutalWorld/build/에 같은 산출물이 두 벌, hw0/build/에 한 벌 더 들어 있다. 루트 .gitignore*/classes가 있는데도 남아 있는 이유는 무시 규칙을 추가하기 전에 이미 커밋했고 git rm --cached를 하지 않았기 때문이다. 커밋 로그에 "Update classes"가 두 번(2017-10-16, 2017-11-28) 등장하는 것으로 보아, 오히려 의도적으로 갱신해 왔다.

IDE 설정도 그대로다. *.iml 4개와 .idea/ 11개가 추적되고, 그중 ParBST/.idea/uiDesigner.xml.gitignore가 명시적으로 제외하는 파일인데도 남아 있다. 더 나쁜 것은 ParBST/parbst.propertieshw0/build.properties다. 전자에는 C:\Users\maybe\.m2\repository, C:/Program Files/Java/jdk1.8.0_144 같은 개인 머신의 절대 경로가 들어 있고, hw0/build.xml은 60행에서 <fileset dir="${jdk.home.1.8}">로 그 경로를 실제로 참조한다. 다른 머신에서는 빌드가 그 자리에서 실패한다.

빌드 구성에 테스트 타깃이 없다.build.xml 전부 srcdir="${src}"만 컴파일하고 test/는 건드리지 않으며, junit 태스크도 없고 JUnit jar도 레포에 없다(.gitignore*.jar를 통째로 무시한다). 테스트 9개는 IntelliJ 안에서만 실행 가능하고, 빌드 파이프라인에서는 존재하지 않는 것과 같다. 소프트웨어공학 과목에서 이 간극은 우연이 아니라 구조적 누락이다.

커밋 이력 자체는 진짜다. 131개가 2017-09-11부터 2017-12-27까지 학기 리듬대로 분포하고, 마감 직전인 11월 하순에 밀집한다(2021-08-09 커밋 하나는 GitHub 계정 rename). 다만 메시지 품질은 낮다 — "Update tests", "Update BST", "Update gitignore"(같은 날 3연속)처럼 무엇이 왜 바뀌었는지 알 수 없는 것이 대다수다. 그 사이에 "Fix search function invalid", "Fix RWBinaryTree hold read lock problem", "Remove debug log"처럼 정확한 메시지가 섞여 있어, 잘 쓸 줄 모르는 게 아니라 대부분의 경우 쓰지 않았다는 쪽에 가깝다. 저자 identity가 6개(배지운, bae jiun, maybe, Bae jiun, Maybe, Jiun Bae, MaybeS, Maydev)로 갈려 있는 것도 같은 성격의 방치다.

반복되는 패턴

  1. 테스트가 실패할 수 없도록 설계되어 있다. 이것이 이 레포의 중심 문제다. RWBinaryTreeTest는 대상을 생성하지 않아 전부 NPE로 죽고, 워커 스레드 안의 assertTruePoolError를 잡지 않아 메인 스레드에 도달하지 않으며, remove/delete의 반환값은 어디서도 단언되지 않고, size()는 검사되지 않으며, hw0의 테스트는 초기 용량 안에서만 논다. 그 위에서 보고서는 "테스트를 모두 통과했으므로 올바르게 작동한다"를 두 번 반복한다. 테스트를 쓰지 않은 사람보다, 통과하는 테스트를 쓰고 그것을 증거로 삼은 사람이 더 위험하다.
  2. 문서가 코드보다 앞서간다. Part 2의 제목을 바꿔 빠진 lock-free BST를 덮었고, "skeleton을 변경하지 않으면서"라고 쓴 뒤 skeleton의 공개 API를 바꿨으며, write()의 javadoc은 재시도 계약을 명시하는데 호출부는 재시도하지 않고, 보고서는 complex 테스트가 "100개의 작업"을 한다고 쓰는데 코드는 100만 번 돈다(BinaryTreeTest.java:88). 문서를 읽고 코드를 예상하면 매번 틀린다.
  3. 행복 경로만 완성되어 있다. BinaryTree.delete는 있는 키만 지울 수 있고, FriendGraph는 16명 16친구까지만 살아 있으며, lockfree.LinkedListInteger로만 정확하다. 경계와 실패 경로에 손이 닿지 않았고, 정확히 그 경로들이 테스트되지 않은 경로와 일치한다.
  4. 실패를 삼키는 습관. catch (RuntimeException e) {}(RWBinaryTree.java:84) 뒤에 return true, catch (Exception e) 뒤에 return null(Actionable.java:450–453), 주석 처리된 e.printStackTrace()(Actionable.java:65), Poolcatch (RuntimeException). 예외가 났다는 사실이 호출자에게 도달하는 경로가 코드 전반에서 차단되어 있다. 위의 1번 패턴과 같은 뿌리다.
  5. 자기 숫자를 의심하지 않는다. 삽입 303,515 ms 직후 같은 작업이 25 ms로 찍혔는데, 보고서는 그것을 lock-free의 장점으로 해석한다. 벤치마크에서 가장 먼저 물어야 할 질문 — "이 숫자가 말이 되는가" — 이 한 번도 등장하지 않는다.

지금 손본다면

  1. PoolExecutorService로 교체하고, 모든 단언을 메인 스레드로 끌어올린다. 작업을 Callable로 제출해 Future.get()으로 결과를 받으면 워커에서 터진 예외가 ExecutionException으로 전달된다. 한 시간짜리 작업이고, 이것만으로 현재 통과 중인 테스트 여러 개가 즉시 빨간색으로 바뀐다 — 그게 목적이다.
  2. RWBinaryTreeTest.makeInstancetree = new RWBinaryTree<>(); 한 줄을 넣는다. 한 줄이다. 이 레포에서 가장 값싼 수정이자, 보고서가 3문단에 걸쳐 설명한 구현이 실제로 동작하는지 처음으로 알게 되는 순간이다.
  3. BinaryTree.delete의 락 획득을 try/finally로 감싸고 117행에 null 검사를 넣는다. 전역 락 영구 보유와 par 락 누수가 함께 사라진다. 이어서 "존재하지 않는 키 삭제" 테스트를 추가한다 — 지금 이 케이스는 세 구현 중 어느 것에도 없다.
  4. attemptMarkcompareAndSet(suc, suc, false, true)로 바꾸고, counterAtomicInteger로 만들어 CAS 성공 지점에서만 증감한다. 그 다음 테스트에서 add/remove의 반환값 합계와 size()가 일치하는지 단언한다. 이 불변식 하나가 lock-free 리스트의 동시성 버그 대부분을 잡아낸다.
  5. getDistancelast를 레벨 카운터로 바꾼다. visit[v] + 1을 이웃에 기록하는 표준 형태면 충분하고, 별도 카운터가 필요 없어진다. 함께 addPerson>>=로, adjust의 확장 분기에 새 행 초기화를, newConnection(size, connection)의 길이 계산을 size + 1로 고친다.
  6. build.xmljunit 타깃을 추가하고 .class 114개와 IDE 설정을 git rm --cached한다. 절대 경로가 박힌 *.properties도 함께 제거하면 다른 머신에서 처음으로 빌드가 성립한다. 소프트웨어공학 과목에서 이건 부수적 정리가 아니라 과제의 일부다.