2 คะแนน โดย GN⁺ 2025-03-01 | 1 ความคิดเห็น | แชร์ทาง WhatsApp
  • ในแอปพลิเคชัน Java 17 คอร์ทั้ง 32 คอร์ถูกใช้งานจนเต็ม ทำให้การใช้งาน CPU พุ่งถึง 3,200% และเมื่อเรียง thread dump ตาม CPU time ก็พบว่ามีหลายเธรดติดอยู่ที่ TreeMap.put()
  • โค้ดลูปที่สงสัยในตอนแรกแม้จะไม่มีประสิทธิภาพ แต่ความซับซ้อน O(N lg(M)) และการตรวจสอบขนาดอินพุตอย่างเดียวไม่สามารถอธิบายเวลารันระดับที่ทำให้ระบบล่มได้
  • สาเหตุจริงคือหลายเธรดแก้ไข TreeMap ที่ใช้ร่วมกันโดยไม่มีการป้องกัน ทำให้ภายใน red-black tree เกิด cycle และการค้นหา/แทรกค่าตกอยู่ในลูปไม่สิ้นสุด
  • โค้ดจำลองแบบง่าย, ExecutorService และบริการ gRPC ต่างก็เกิดอาการเดียวกัน และ thread pool ไม่แสดง NPE ออกทาง standard output ทำให้หาสาเหตุยากขึ้น
  • การแก้ไขไม่ได้จบแค่เปลี่ยนไปใช้ Collections.synchronizedMap หรือ ConcurrentHashMap แต่ควรมีการจัดการ exception ของ executor, การแจ้งเตือน CPU/NPE, static analysis และการทดสอบแบบมัลติเธรดร่วมด้วย

อาการของเหตุขัดข้องและเบาะแสแรก

  • เครื่องเสียหายหนักจนแทบ ssh เข้าไปไม่ได้ และการใช้งาน CPU ขึ้นไปถึง 3,200%
    • อยู่ในสภาพที่ คอร์ทั้ง 32 คอร์ของโฮสต์ถูกใช้งานทั้งหมด
    • เป็นสถานการณ์ที่ต่างจากบั๊กก่อนหน้าที่ใช้คอร์เดียว 100%
  • thread dump ของ runtime Java 17 มี CPU time รวมอยู่ด้วย และเมื่อเรียงตาม CPU time ก็เห็นเธรดที่มีลักษณะคล้ายกันหลายตัว
    • stack ชี้ไปที่ java.util.TreeMap.put()
    • ตำแหน่งในโค้ดแอปพลิเคชันคือ BusinessLogic.someFunction(BusinessLogic.java:29)

โค้ดที่สงสัยในตอนแรกและสมมติฐานที่ถูกตัดออก

  • โค้ดในจุดที่เป็นปัญหาวนผ่าน unrelatedObjects แต่ในตัวลูปกลับใช้เพียง relatedObject
for (SomeOtherType unrelatedObject : unrelatedObjects) {
    treeMap.put(relatedObject.a(), relatedObject.b()); // line 29
}
  • โค้ดนี้สามารถย่อให้เหลือ put เพียงครั้งเดียวได้ดังนี้
treeMap.put(relatedObject.a(), relatedObject.b());
  • เนื่องจาก unrelatedObjects ถูกใช้ในช่วงท้ายของฟังก์ชัน จึงไม่สามารถลบพารามิเตอร์นี้ออกได้
  • เป็นไปได้ว่าการใช้งาน unrelatedObject หายไประหว่างกระบวนการ refactor
  • ใน unit test แม้เพิ่มขนาด treeMap และ unrelatedObjects เป็นอย่างละ 1,000,000 entry ก็ยังจำลองปัญหาไม่ได้
    • หากให้ขนาดของ unrelatedObjects เป็น N และขนาดของ treeMap เป็น M ความซับซ้อนคือ O(N lg(M))
    • ประเมินว่าหากจะเห็นเวลารันระดับ 1 นาที ต้องมี entry ระดับ 100 ล้านถึง 1 พันล้าน
    • ซึ่งไม่สอดคล้องกับสมมติฐานว่าในแอปพลิเคชันจริง โครงสร้างข้อมูลทั้งสองมีขนาดไม่เกิน 1,000

ลูปไม่สิ้นสุดที่เกิดจาก TreeMap ที่ไม่มีการป้องกัน

  • นิยามฟิลด์ TreeMap เป็นดังนี้
private final Map<K,V> treeMap = new TreeMap<>();
  • มีหลายเธรดเข้าถึง TreeMap นี้ แต่ไม่มี synchronization หรือกลไกป้องกันใด ๆ
  • TreeMap ของ Java ถูก implement ด้วย red-black tree และหากการแก้ไขพร้อมกันทำให้การเชื่อมโยง node ภายในเสียหาย ก็อาจเกิด cycle ได้
  • ระหว่างค้นหาหรือใส่ค่าที่ยังไม่มี อาจไล่ตาม cycle และตกอยู่ใน ลูปไม่สิ้นสุด

การทดลองจำลองแบบง่าย

  • เขียนการทดลองให้หลายเธรดอัปเดต TreeMap ที่ใช้ร่วมกันแบบสุ่ม
for (int i = 0; i < numThreads; i++) {
    threads.add(new Thread(() -> {
        Random random = new Random();
        for(int j = 0; j < numUpdates; j++) {
            try {
                treeMap.put(random.nextInt(1000), random.nextInt(1000));
            } catch (NullPointerException e) {
                // let it keep going so we can reproduce the issue.
            }
        }
    }));
}
  • โปรเจกต์ทั้งหมดอยู่ที่ SimpleRepro.java
  • ตอนแรก try/catch ดูเหมือนเป็นหัวใจสำคัญ
    • ถ้าไม่มี try/catch เธรดบางตัวจะตายด้วย NullPointerException และโปรแกรมหยุดทำงาน
    • หลังใส่ try/catch และรันไม่กี่ครั้ง ก็พบ การใช้งาน CPU 500%
  • race condition ไม่ได้ทำให้เกิดเพียงข้อมูลเสียหายหรือ deadlock เท่านั้น แต่ยังอาจทำให้โครงสร้างข้อมูลเสียจนอยู่ในรูปที่เกิดลูปไม่สิ้นสุดได้ และนำไปสู่ ปัญหาด้านประสิทธิภาพ

การยืนยัน cycle ภายใน TreeMap

  • เขียนการทดลองที่ใช้ reflection เข้าถึงฟิลด์ root, left, right, key, color ของ TreeMap เพื่อพิมพ์ node และสีออกมา
  • ระหว่าง traversal หากพบ TreeMap.Entry ที่เคยเยี่ยมชมแล้วอีกครั้ง จะถือว่าเป็น cycle
private void print(
    Object treeMapEntry, String tabs, IdentityHashMap<Object, Object> visited
) throws Exception {
    if (treeMapEntry != null && !visited.containsKey(treeMapEntry)) {
        visited.put(treeMapEntry, treeMapEntry);
        print(treeMapEntryLeft.get(treeMapEntry), tabs + " ", visited);
        System.out.println(tabs + treeMapEntryKey.get(treeMapEntry) + ":"
            + (treeMapEntryColor.getBoolean(treeMapEntry) ? "BLACK" : "RED"));
        print(treeMapEntryRight.get(treeMapEntry), tabs + " ", visited);
    } else if (treeMapEntry != null && visited.containsKey(treeMapEntry)) {
        System.out.println(tabs + treeMapEntryKey.get(treeMapEntry) + ":"
            + (treeMapEntryColor.getBoolean(treeMapEntry) ? "BLACK" : "RED")
            + " CYCLE"
        );
    }
}
  • เพื่อเปิดข้อจำกัดการเข้าถึงของ Java module ต้องใช้ JVM argument ต่อไปนี้ตอนรัน
--add-opens java.base/java.util=ALL-UNNAMED

กรณีที่เกี่ยวข้องในอดีตและความแตกต่าง

การจำลองที่สมจริงขึ้นด้วย ExecutorService

  • โค้ดที่แค่ละเลย NPE อาจดูไม่สมจริง แต่ใน ExecutorService exception อาจไม่แสดงให้เห็นได้ง่าย
final ExecutorService pool = Executors.newFixedThreadPool(numThreads);
final TreeMap<Integer,Integer> treeMap = new TreeMap<>();
Random random = new Random();

for (int i = 0; i < numThreads*numUpdatesPerThread; i++) {
    pool.submit(() -> {
        treeMap.put(random.nextInt(10000), random.nextInt(10000));
    });
}
  • โค้ดทั้งหมดอยู่ที่ ExecutorUncaughtRepro.java
  • เมื่อรัน โปรแกรมจะหยุดค้าง และ thread dump จะแสดงเธรดที่ติดอยู่ใน TreeMap.put()
  • ไม่มีอะไรออกมาทาง standard output
    • thread pool กลืน NPE ทำให้สัญญาณของปัญหาไม่ปรากฏ
    • สถานการณ์จริงก็เป็นเช่นนี้
  • หากจัดการ thread pool เอง จำเป็นต้องมีมาตรการต่อไปนี้
    • ลงทะเบียน uncaught exception handler ผ่าน thread factory
    • จัดการ Future ที่ส่งคืนมา และใช้ future.get() เพื่อตรวจสอบ NPE ที่ถูกห่ออยู่ใน ExecutionException

ปัญหาเดียวกันในบริการ gRPC

  • ในบริการที่ใช้ thread pool เป็นฐานอย่างบริการ gRPC TreeMap ที่ไม่มีการป้องกันก็สามารถสร้างปัญหาเดียวกันได้
@Override
public void addReceipt(
    ReceiptProcessorServiceOuterClass.AddReceiptRequest req,
    StreamObserver<ReceiptProcessorServiceOuterClass.AddReceiptResponse> responseObserver
) {
    int timestamp = req.getTimestamp();
    int totalPrice = req.getTotalPrice();
    receipts.put(timestamp, totalPrice);
    ReceiptProcessorServiceOuterClass.AddReceiptResponse response =
        ReceiptProcessorServiceOuterClass.AddReceiptResponse.newBuilder().build();
    responseObserver.onNext(response);
    responseObserver.onCompleted();
}
  • โค้ดทั้งหมดอยู่ที่ GrpcRepro.java
  • นิยาม protobuf อยู่ที่ ReceiptProcessorService.proto
  • thread dump แสดงให้เห็นว่าเธรด grpc-default-executor-* หยุดค้างในสถานะ runnable ที่ TreeMap.put()

ข้อสันนิษฐานเกี่ยวกับสาเหตุและการทดลองรายภาษา

  • สงสัยว่าสาเหตุที่เป็นไปได้คือเธรดสองตัวหมุน tree ไปคนละทิศอย่างอิสระ หรือการเขียนของการหมุนที่ทับซ้อนกันเกิด interleaving จนสร้าง cycle
  • อย่างไรก็ตาม ยังไม่มีการพิสูจน์ว่า interleaving ของเธรดแบบใดทำให้เกิด cycle
  • ตอนแรกสงสัยว่าจำเป็นต้องมี NPE แต่การทดลองภายหลังทำให้สมมติฐานนี้ถูกล้มล้าง
  • พยายามจำลองปัญหาเดียวกันในหลายภาษา
    • Java เป็นฐานของกรณีทั้งหมด จึงจำลองได้
    • std::map ของ C++ ใช้ red-black tree และพบไม่บ่อยที่แทนที่จะ segfault เธรดกลับค้างและใช้ CPU สูง
    • Go ก็จำลองได้อย่างเหนือความคาดหมาย และโค้ดอยู่ที่ การทดลอง Go
    • Ruby แม้เป็นภาษาที่สามารถจับ NPE ได้ก็จำลองไม่ได้ และเป็นไปได้ว่า GIL ป้องกัน interleaving ที่ทำให้เกิดปัญหา
  • การทดลอง C++ จำลองลูปไม่สิ้นสุดได้โดยไม่ต้องมี try/catch หรือ exception จาก null pointer ซึ่งผิดจากที่คาด
    • บางครั้งจบด้วย segmentation fault
    • นาน ๆ ครั้ง โปรแกรมหยุดค้างเกิน 10 นาที และใน top เห็น SimpleRepro ใช้ CPU 170.8%
    • ใน C++ การอ้างอิง null pointer คือ segfault ดังนั้นต้องมี interleaving ที่ไม่ผ่าน null อยู่
  • หลังเห็นผลนี้ จึงกลับไปรันการทดลอง Java อีก 12 ครั้งโดยไม่จับ NPE และจำลองลูปไม่สิ้นสุดใน TreeMap.put() ได้แม้ไม่มี NPE catch

วิธีแก้ง่าย ๆ และการป้องกันในระดับโครงสร้างข้อมูล

  • วิธีแก้ที่ง่ายที่สุดคือปกป้อง TreeMap ที่ใช้ร่วมกัน
    • ห่อด้วย Collections.synchronizedMap
    • เปลี่ยนเป็น ConcurrentHashMap แล้วค่อยจัดเรียงเฉพาะเมื่อจำเป็น
  • การป้องกันที่ถกเถียงได้มากกว่าคือการติดตาม node ที่เคยเยี่ยมชม ระหว่าง traversal ของ red-black tree
    • หากพบ node ที่เคยเยี่ยมชมแล้วอีกครั้ง ให้โยน ConcurrentModificationException
    • ไม่ได้ป้องกันข้อมูลเสียหายเอง
    • แต่ป้องกันการใช้ CPU 100% จากลูปไม่สิ้นสุดได้
    • หน่วยความจำเพิ่มเติมถูกจำกัดตามความสูงของ tree คือ O(lg(n))
    • ความสูงของ red-black tree รับประกันว่าเป็น O(lg(n))
    • มองว่าโอกาสที่ standard library จะนำวิธีนี้ไปใช้นั้นต่ำ
  • ตัวอย่างการแก้ไขบันทึก node ที่เคยเยี่ยมชมด้วย IdentityHashMap ใน getEntry และ put
IdentityHashMap<Entry<?,?>, Boolean> visited = new IdentityHashMap<>();

while (p != null) {
    visited.put(p, true);
    int cmp = k.compareTo(p.key);
    if (cmp < 0)
        p = p.left;
    else if (cmp > 0)
        p = p.right;
    else
        return p;

    if (visited.containsKey(p)) {
        throw new ConcurrentModificationException("TreeMap corrupted. Loop detected");
    }
}

สร้างชั้นป้องกันหลายชั้น

  • ความผิดพลาดเกิดขึ้นได้ ดังนั้นการป้องกันเพียงชั้นเดียวไม่เพียงพอ

  • ในกรณีนี้มีความผิดพลาดหลายอย่างซ้อนกัน แต่ยังเหลือการเฝ้าระวังบางส่วนทำให้พบปัญหาได้

  • การแจ้งเตือน NPE

    • ไม่มีการแจ้งเตือนสำหรับการเกิด NPE เอง
    • มีเพียงการแจ้งเตือนอัตราข้อผิดพลาด และ NPE นี้เกิดเพียงครั้งเดียวต่อ API handler worker thread จึงไม่เกิน threshold ของอัตราข้อผิดพลาด
    • ด้วยวิธีจัดการของ executor จึงไม่เหลือ log ของ NPE ไว้ด้วย
  • การแจ้งเตือนความผิดปกติของการใช้งาน CPU

    • มีการ monitor การใช้งาน CPU และใช้การแจ้งเตือนตาม threshold แบบง่าย
    • เมื่อการใช้งาน CPU เกิน threshold ก็ถูกมองว่าเป็นพฤติกรรมผิดปกติและเกิดการแจ้งเตือน ทำให้พบปัญหาผ่านเส้นทางนี้
  • การจัดการ exception ของ Executor

    • หากส่งงานเข้า executor ต้องตั้งค่า uncaught exception handler เสมอ
    thread.setUncaughtExceptionHandler(
        (dyingThread, throwable) -> {
            logger.error("uncaught exception!", throwable);
        }
    );
    
    • แทนที่จะเขียน ThreadFactory เอง สามารถใช้ Apache Commons BasicThreadFactory หรือ Guava ThreadFactoryBuilder ได้
    • หากไม่มี handler exception จะถูกกลืน และ log monitoring ก็ตรวจไม่พบปัญหา
  • Code review และ static analysis

    • ใน code review อาจพบการใช้เธรดร่วมกับ TreeMap หรือมีข้อเสนอว่าไม่ควรใช้ TreeMap หากไม่จำเป็นต้องเรียงลำดับ แต่ในกรณีนี้ไม่เกิดขึ้น
    • เครื่องมือ static analysis เช่น SpotBugs, JLint, Chord สามารถค้นหาปัญหา concurrency ประเภทนี้ได้ตั้งแต่ช่วง build
    • มีการยกเอกสารเกี่ยวกับ static analysis คือ How Good is Static Analysis at Finding Concurrency Bugs?
  • การทดสอบ

    • ไม่มี การทดสอบแบบมัลติเธรด สำหรับ code path นี้
    • หากเป็นโค้ดที่อาจถูกเข้าถึงพร้อมกัน การทดสอบก็ควรครอบคลุมสถานการณ์แบบมัลติเธรดด้วย

สรุป

  • การแก้ไขพร้อมกันโดยไม่มีการป้องกันไม่ได้ก่อให้เกิดแค่ข้อมูลเสียหาย แต่ยังอาจปรากฏเป็น ลูปไม่สิ้นสุด และการใช้งาน CPU สูง
  • หากหลายเธรดแก้ไขโครงสร้างข้อมูลที่มีโครงสร้าง pointer ภายในอย่าง TreeMap พร้อมกัน โครงสร้าง red-black tree อาจสร้าง cycle ได้
  • หากมีการเลือกโครงสร้างข้อมูลที่ synchronized, การจัดการ exception, การแจ้งเตือน, static analysis และการทดสอบแบบมัลติเธรดร่วมกัน จะช่วยให้พบหรือป้องกันปัญหาได้เร็วขึ้น

1 ความคิดเห็น

 
GN⁺ 2025-03-01
ความคิดเห็นจาก Hacker News
  • เป็นที่รู้กันดีว่า collection หลัก ๆ ของ Java นั้นตามการออกแบบแล้ว ไม่ปลอดภัยต่อเธรด ดังนั้นเรื่องนี้ควรถูกพบตั้งแต่แรก
    OP ควรกวาดดูโค้ดส่วนที่เหลือด้วยว่า ยังมีจุดอื่นที่หลายเธรดอาจจัดการ collection พร้อมกันอยู่หรือไม่
    การห่อด้วย Collections.synchronizedMap หรือเปลี่ยนเป็น ConcurrentHashMap จะทำให้การดำเนินการกับ map รายครั้งปลอดภัยต่อเธรด แต่ก็เป็นอีกเรื่องหนึ่งว่า ชุดของการดำเนินการต่อเนื่องกัน จะปลอดภัยด้วยหรือไม่
    ตัวอ็อบเจ็กต์ที่เป็นเจ้าของ TreeMap เองก็น่าสงสัยว่าปลอดภัยต่อเธรดหรือไม่ และการแก้แบบติดตามโหนดที่เยี่ยมชมจะยิ่งทำให้ collection ยังไม่ปลอดภัยอยู่เหมือนเดิม แต่ล้มเหลวในรูปแบบที่ละเอียดอ่อนกว่าเท่านั้น
    ประเด็นหลักไม่ใช่ side effect ของ TreeMap แต่เป็นการละเมิดสัญญาของ collection และแม้จะเปลี่ยนเป็น HashMap โค้ดนี้ก็ยังผิดอยู่ดี
    ในโค้ดที่มีหลายเธรดเข้ามาจัดการ วิธีที่แน่นอนที่สุดคือทำให้อ็อบเจ็กต์ทุกตัวที่เป็นไปได้เป็น immutable และสำหรับอ็อบเจ็กต์ที่ทำให้ immutable ไม่ได้ ก็จำกัดให้อยู่เฉพาะในพื้นที่เล็ก ๆ เป็นอิสระ และถูกควบคุมอย่างเข้มงวด

    • จุดนี้สำคัญมาก: ต่อให้มี dynamic array ที่ปลอดภัยต่อเธรด การผสมสองปฏิบัติการอย่างตรวจความยาวด้วย size() แล้วเข้าถึง element(10) ก็ ไม่เป็นอะตอมมิก
      ถ้ามีเธรดอื่นลบ element ระหว่างการเรียกสองครั้งนั้น ก็อาจเกิดการเข้าถึงนอกขอบเขตได้
      ทางแก้คือใช้เมธอดที่จัดการสองอย่างร่วมกันแบบอะตอมมิก เช่น element_or_null() หรือไม่ก็ใช้ array ธรรมดาตั้งแต่แรก แล้วป้องกันทั้งสองปฏิบัติการด้วย mutex
    • data race เกิดจาก สถานะเปลี่ยนแปลงได้ที่แชร์ร่วมกัน โดยไม่มีการซิงโครไนซ์
      ต้องมีเงื่อนไขครบทั้งสามอย่าง ดังนั้นทางแก้ก็มีสามแบบ: เพิ่มการซิงโครไนซ์ด้วย lock เป็นต้น, ไม่แชร์การเข้าถึงแบบเปลี่ยนแปลงได้ เช่นโมเดลเจ้าของเดียวที่ใช้ channel, หรือทำข้อมูลให้เป็น immutable ตาม insight จากภาษา pure functional
      เท่าที่รู้ Google เองก็ลงทุนกับโมเดลนี้พอสมควรใน Guava
      Rust ให้เราเลือกว่าจะยอมละทิ้งข้อใดในสามข้อนี้ และป้องกันแบบ static ไม่ให้เงื่อนไขทั้งสามเกิดขึ้นพร้อมกัน
    • ความคิดแรกที่ผุดขึ้นมาก็คือ “เดี๋ยวนะ data structure นั้นปลอดภัยต่อเธรดหรือเปล่า?” และสุดท้ายดูเหมือนเป็น การเลือก data structure ที่ไม่เหมาะกับวัตถุประสงค์
      ถ้าฝืนยัด mutex เข้าไปให้พอดี ก็มักจะเกิดปัญหาอื่นและคอขวดตามมา
      หากจะใช้อ็อบเจ็กต์ immutable ก็ควรใช้ immutable data structure ที่อาศัย structural sharing เพื่อหลีกเลี่ยงหรือลดการใช้หน่วยความจำที่พุ่งสูง
      ถ้าใช้ pure functional data structure ปัญหาด้านหนึ่งจะหายไป แต่ถ้าเธรดต่าง ๆ พึ่งพาเวอร์ชัน промежуточная ที่อีกฝ่ายสร้างขึ้น ก็จะเกิดปัญหาปวดหัวอีกแบบ และอาจต้องใช้ data structure อื่น
      หากจะฝืนรักษา mutable data structure ที่ใช้อยู่แล้วไว้ ก็อาจ serialize ความพยายามเข้าถึงก่อนที่จะไปแตะ data structure และมัดการเข้าถึงเป็น transaction เพื่อให้รันเฉพาะ transaction ที่สมบูรณ์เท่านั้น แต่ทำไปทำมาก็แทบจะเหมือนกำลังสร้างฐานข้อมูลขึ้นมาเอง
    • ผมมองว่า โมเดล threading ที่พยายามรันอย่างอิสระในสภาพแวดล้อม shared memory นั้นผิดตั้งแต่รากฐาน
      คิดว่าความพยายามที่จะทำให้สิ่งนั้นทำงานได้ควรถูกนำไปลงกับโมเดล process ที่ดีกว่านี้มากกว่า
  • ตัวอย่างที่ทำให้โค้ดง่ายลงในต้นฉบับนั้นไม่แม่นยำ
    โค้ดเดิมจะทำ treeMap.put เฉพาะเมื่อ unrelatedObjects ไม่ว่าง ซึ่งความต่างนี้อาจเป็นบั๊กหรือไม่ก็ได้
    นอกจากนี้ต้องตรวจด้วยว่า a และ b คืนค่าเดิมทุกครั้งหรือไม่ และ treeMap ทำงานเหมือน map จริง ๆ เท่านั้นหรือเปล่า
    เช่น ถ้าเป็น map ที่บันทึกการอัปเดตลง log ก็ต้องพิจารณาว่าการเปลี่ยนให้บันทึก log แค่ครั้งเดียวทำได้จริงหรือไม่

    • เป็นข้อทักท้วงที่ถูกต้อง ควรเปลี่ยนเป็น if ที่ตรวจว่าไม่ว่าง
  • ถ้าการ implement Comparator หรือ Comparable ไม่ได้ให้ total order ที่สอดคล้องกัน ก็อาจเกิดลูปไม่รู้จบได้เช่นกัน: https://stackoverflow.com/questions/62994606/concurrentskips...
    เรื่องนี้ไม่เกี่ยวกับ concurrency
    จะเกิดหรือไม่อาจขึ้นอยู่กับข้อมูลเฉพาะที่ถูกประมวลผลและลำดับการประมวลผล จึงอาจดูเหมือนทำงานปกติอยู่นาน แล้วไประเบิดใน production ได้

    • สงสัยว่าเคยเห็นกับตาจริง ๆ ไหม น่าจะเป็นหัวข้อบล็อกที่ดี
      ส่วนตัวผมยังไม่เคยเจอ comparator ที่มีบั๊กแบบไม่มี total order
  • เคยคิดว่า race condition ทำให้เกิดแค่ข้อมูลเสียหายหรือ deadlock เท่านั้น ไม่เคยคิดว่าจะทำให้เกิด ปัญหาประสิทธิภาพ ได้
    แต่ถ้ามันทำให้ข้อมูลเสียจนเกิดลูปไม่สิ้นสุดได้ ก็สมเหตุสมผล
    เลยมักคิดอยู่เสมอว่าโดยหลักการแล้ว ข้อผิดพลาด พฤติกรรมผิดปกติ และคำเตือนในโปรเจกต์ควรถูกแก้ เพราะมันอาจก่อปัญหาที่ดูเผิน ๆ ไม่เกี่ยวข้องกันได้
    เพียงแต่หลักการแบบนี้ไม่ค่อยได้รับการยอมรับจากฝ่ายที่ตัดสินใจว่าจะทำอะไร

    • เป็นกฎจากประสบการณ์ที่ใช้ได้ แต่ต้องมี ความเป็นจริงเชิงปฏิบัติ ด้วย
      ในบางโปรเจกต์ ค่าใช้จ่ายในการกำจัดข้อผิดพลาด พฤติกรรมผิดปกติ และคำเตือนทั้งหมด อาจสูงกว่าค่าใช้จ่ายในการยอมรับปัญหาที่ดูไม่เกี่ยวข้องซึ่งเกิดขึ้นเป็นครั้งคราวมาก
      แทบคาดเดาไม่ได้เลยว่าข้อผิดพลาดหนึ่ง ๆ จะเกี่ยวข้องกับปัญหาในอนาคตหรือไม่ และการแก้ไว้ล่วงหน้าจะถูกกว่าหรือเปล่า ดังนั้นสุดท้ายจึงใกล้เคียงกับงานช่างมากกว่าวิทยาศาสตร์
      “ไม่แก้อะไรเลย” นั้นแย่มาก และ “แก้ทุกอย่าง” ก็มักไม่สมจริง จึงต้องมีกรอบการตัดสินใจ สัญชาตญาณจากประสบการณ์ และความไว้วางใจจากผู้มีส่วนได้ส่วนเสีย
    • ที่ทำงานถือว่า คำเตือนทั้งหมดเป็นข้อผิดพลาด และจะ merge pull request ไม่ได้ถ้า CI pipeline ทั้งหมดที่รันอัตโนมัติไม่ผ่าน
      ต้องมีวินัย แต่พอทำให้ถึงสภาพนั้นได้แล้ว การรักษาไว้จะง่ายกว่ามาก
    • การไม่จับ exception แล้วละทิ้งไปก็สำคัญเช่นกัน
      ถ้าไม่ได้รู้อย่างชัดเจนว่าการให้โปรแกรมทำงานต่อไปนั้นปลอดภัย “จับแล้วแค่เขียน log” ก็เป็นความคิดที่ไม่ดี
      ปล่อยให้ exception ไหลต่อไปจนถึงจุดที่จัดการได้อย่างมีประโยชน์ เช่นคืนค่า 500 หรือแสดงกล่องโต้ตอบข้อผิดพลาด จะดีกว่า
    • ท้ายที่สุดต้องเลือกสนามที่จะสู้
      เคยมีปัญหาในโปรเจกต์ Rails ที่ดูแลอยู่ ซึ่ง log เต็มไปด้วย “unsupported parameters” แม้จะตรวจ controller อย่างละเอียดและอนุญาตแล้วก็ยังขึ้นอยู่
      น่าจะไม่มีอันตราย แต่ทำให้ log มี noise เยอะ
      หลายคนพยายามแก้แล้ว แต่ก็มีเรื่องที่ลำดับความสำคัญสูงกว่าเสมอ และยากจะอธิบายเหตุผลในการใช้เวลามากกับงานที่ไม่มีผลต่อฟีเจอร์
      เป็นปัญหาน่ารำคาญเหมือนริดสีดวงทวาร ต้องเลือกระหว่างผ่าตัดแล้วเจ็บหนักไปหลายสัปดาห์ หรือทนอยู่กับมันไป โดยส่วนใหญ่มันไม่ร้ายแรง แต่ก็มีโอกาสจะหนักขึ้นจนเป็นปัญหาใหญ่
      ปรากฏการณ์แบบนี้น่าจะเรียกว่า “ริดสีดวงดิจิทัล” ได้
    • แม้ไม่มีความเสียหาย race condition ก็อาจทำให้งานเดียวกันถูกทำซ้ำหลายครั้งและเหลือผลลัพธ์ไว้แค่หนึ่งชุด จนเกิด ปัญหาประสิทธิภาพใหญ่ ได้
      ถ้าตัดสินแล้วว่าคำเตือนไม่เกี่ยวข้อง อย่างน้อยควรอธิบายไว้ใน comment และถ้าเป็นไปได้ควรใส่ pragma เพื่อปิดคำเตือนในขอบเขตที่แคบที่สุด
      ส่วนพฤติกรรมผิดปกติ ผมชอบกำจัดมันมากกว่า เคยเจอโค้ดที่ถูกทำเครื่องหมายไว้ว่า “ไม่รู้ว่าทำไมถึงทำงานได้” แล้วภายหลังมันก็ไม่ทำงานอีกต่อไป ถ้าเก็บกวาดไว้ก่อนก็คงมีเวลาค่อย ๆ แก้อย่างรอบคอบ แต่สุดท้ายต้องรีบเขียนใหม่
  • พอเห็นส่วนที่ว่า “แทบจะ ssh เข้าไปได้” ก็ทำให้นึกถึงสถานการณ์สมัยเรียนบัณฑิตศึกษา
    แล็บของเราเป็นกลุ่มเล็ก ๆ ด้านการประมวลผลแบบขนานและแบบกระจาย มีคนประมาณ 8 คนใช้เครื่องร่วมกัน ซึ่งน่าจะเป็น Sun UltraSparc 170 มีฮาร์ดดิสก์ 1GB และ RAM ระดับ 128 หรือ 256MB
    ต้องคำนึงด้วยว่าเครื่อง Sun แทบไม่ค่อย reboot
    นักศึกษาใหม่คนหนึ่งดูเหมือนจะแบ่งไฟล์ข้อความขนาดใหญ่เป็น 32 หรือ 64 ช่วงตามเลขบรรทัด แล้วไม่สร้างไฟล์แยก แต่รันสำเนา perl จำนวน N ตัวแบบขนาน ให้แต่ละตัวประมวลผลช่วงบรรทัดของตัวเอง
    ตามมาตรฐานตอนนั้น RAM จำนวนมากถูกกินโดย perl interpreter จำนวน N ตัว และพอเกิด swap มันก็ seek ไปยังช่วงต่าง ๆ ของไฟล์เดียวกันอย่างบ้าคลั่ง ขณะที่แต่ละ process พยายามอ่านเพิ่มอีกไม่กี่บรรทัด
    ยิ่งกว่านั้น process ตัวที่ J ก็น่าจะต้องอ่านไฟล์ไปถึง J/N เพื่อไปยังช่วงของตัวเอง
    ที่ console ถึงขั้นรับ prompt สำหรับ login ไม่ได้ แต่โชคดีที่มี session ที่ login ไว้แล้ว และ su แสดง prompt รหัสผ่านหลังจากผ่านไป 20–30 นาที
    อีก 5–10 นาทีต่อมาก็ได้ session root ใช้ top ตรวจสาเหตุ จากนั้นพยายามติดต่อผู้ใช้และฆ่า process ที่มีปัญหา ระบบจึงกลับมาเป็นปกติ
    ไอเดียโดยตัวมันเองถูกต้อง แต่เป็นกรณีที่ไม่เข้าใจขีดจำกัดของระบบเลย และเพราะฮาร์ดดิสก์กับ RAM ต่ำ คอขวด I/O จึงรุนแรงมาก การประมวลผลแบบเชิงเส้นธรรมดาน่าจะดีกว่ามาก

  • ไม่ว่าจะเป็น Java หรือภาษาอื่น ๆ ถ้าทำ operation พร้อมกันบน อ็อบเจ็กต์ที่ไม่ thread-safe ก็จะได้บั๊กที่น่าสนใจที่สุดในโลก

    • ดังนั้นเมื่อใช้อ็อบเจ็กต์ที่ไม่ thread-safe จากหลาย thread ต้องจัดการการเข้าถึงแบบ atomic เอง หรือใช้เวอร์ชันที่ thread-safe
      ข้อผิดพลาดจาก multithreading เป็นสิ่งที่ debug แย่ที่สุด
      กรณีนี้ควรระบุได้ง่ายมากตั้งแต่ช่วงออกแบบ และทันทีที่คิดจะใช้ container ทั่วไปในสภาพแวดล้อม multithread ก็ควรมีไฟเตือนติดแล้ว
    • เคยเจอบั๊ก concurrency มาพอสมควร แต่ไม่เคยตั้งใจทำซ้ำความไม่สอดคล้องที่เกิดจากการเอา modifier volatile ออกจากฟิลด์แบบ mutable ของ Java
      JVM ที่ใช้ตอนนั้นอาจถูกสร้างมาดีเกินไปก็ได้
    • แม้ในตอนที่มั่นใจว่าสามารถยอมรับผลลัพธ์จากการแชร์สิ่งที่ไม่ได้ synchronize ได้ ผลลัพธ์จริงก็มักผิดจากที่คาดเสมอ
    • เป็นความคิดที่ค่อนข้างสุดโต่ง แต่ด้วยเหตุผลนี้ผมจึงชอบ debug race ใน C/C++ มากกว่า
      ภาษากำหนด semantic ที่บ้าคลั่งและแทบไม่มีความหมายเมื่อเกิด race ก็จริง แต่ในทางปฏิบัติมักเผยปัญหา memory corruption หรือปัญหาที่มีเสียงดังออกมาบ่อย
      เพราะ race ส่วนใหญ่ถือว่าผิดกฎ จึงสามารถสร้างเครื่องมืออย่าง thread checker ได้โดยไม่ต้องเปลี่ยนซอร์สโค้ดเพื่อระบุความหมาย
      ในทางกลับกัน Java ไม่มี undefined behavior แต่เกิดกรณีที่ฟิลด์สองตัวคลาดเคลื่อนกันอย่างละเอียดอ่อนได้ง่าย และสิ่งเหล่านี้ตามรอยได้ยากกว่ามาก
    • operation บางส่วน หรืออาจเป็นส่วนใหญ่ของ Java Collections ทำการตรวจสอบความถูกต้องภายในเพื่อเตือนปัญหาแบบนี้
      เช่น map โยน ConcurrentModificationException
  • เคยเห็นปรากฏการณ์เดียวกันใน C# ที่รันใน production
    อาการเหมือนกัน และเมื่อดู process dump ก็พบ dictionary ที่เสียหาย
    คิดว่าใช้ ConcurrentDictionary ทั้งหมดแล้ว แต่ตัวที่มาจาก library ตัวหนึ่งเป็นปัญหา
    ตอนนั้นใช้ .NET Framework และเท่าที่จำได้ .NET Core มีโค้ดตรวจจับการแก้ไขพร้อมกัน
    ไม่รู้รายละเอียด implementation แต่แค่มี version counter ก็น่าจะทำได้
    แปลกที่ยึดติดกับ NPE ราวกับเป็นส่วนประกอบสำคัญ เดิมทีรูปแบบการเกิดปัญหาดูเหมือนไม่มีสิ่งนั้น และไม่มีเหตุผลที่จะมองว่า C ไม่มีบั๊กแบบนี้เพียงเพราะไม่มี exception
    แก่นคือ class invariant โดยทั่วไปในระหว่างที่ mutator กำลังทำงาน invariant จะยังไม่เป็นจริง และจะถูกกู้คืนตอนท้ายเท่านั้น
    ถ้าเรียก mutator อีกตัวก่อนที่ invariant จะถูกกู้คืน data structure ก็จะเสียหาย ถ้าไม่ได้เริ่มจากสถานะที่ถูกต้อง ก็มีโอกาสน้อยที่ตอนจบจะเป็นสถานะที่ถูกต้อง

    • สุดท้ายเป็นเพราะตรรกะที่ผิด
      เจอโชคร้ายที่ทำซ้ำไม่ได้ด้วย NPE ที่ไม่ถูกจับ จึงสรุปผิดว่า NPE ที่ไม่ถูกจับเป็นเงื่อนไขจำเป็น
  • ผมเคยเห็นแบบเดียวกันใน java.util.HashMap ที่ขาดการซิงโครไนซ์เหมือนกัน
    เป็นราวปี 2009 แต่เท่าที่รู้ตอนนี้ก็ยังเป็นไปได้
    เท่าที่จำได้ HashMap ใช้ chaining ในการแก้ collision และน่าจะเกิดวงวนขึ้นภายใน chain
    แต่ตอนนั้นไม่ได้มุ่งขุดลงไปเพื่อตรวจสอบยืนยัน แต่เน้นลบโค้ดที่มีปัญหาออกทั้งก้อนมากกว่า
    ความรู้เรื่อง concurrency มักถูกถามในการสัมภาษณ์บ่อย ๆ และถ้าคิดว่า data race เป็นแค่ปัญหาเล็กน้อย ก็คงสร้างความประทับใจได้ยาก กรณีนี้คือเหตุผลว่าทำไม

    • ไม่รู้มาก่อนว่าเรื่องแบบนี้เกิดกับ HashMap ได้ด้วย
      เกี่ยวกับ linked list ที่ใช้จัดการ collision หรือเปล่า?
  • ผมสงสัยว่าวิธีใส่ตัวนับที่เพิ่มขึ้นเพื่อหาวงวน แล้วถ้ามันเกินความลึกของ tree หรือขนาดของ collection ก็โยน exception ออกมา จะเป็นอย่างไร
    ต่างจากวิธี hash set ที่ผู้เขียนเสนอ วิธีนี้แทบไม่มี overhead ด้านหน่วยความจำหรือ CPU และดูมีโอกาสถูกยอมรับมากกว่า
    อย่างไรก็ตาม ใช้ C# มานานกว่า 10 ปี ผมก็ไม่เคยพลาดที่จะพิจารณาการเข้าถึง data structure พร้อมกันในสถานการณ์ concurrent เลย

    • ไม่ใช่ไอเดียที่แย่ แต่โดยทั่วไปการเพิ่ม precondition ที่มีประโยชน์ในสถานการณ์ที่มี data race นั้นค่อนข้างยาก
      tree อาจพังได้อีกหลายแบบนอกจากแบบนี้
    • เป็นไอเดียที่ดี เคยทำมาก่อนกับ binary search และโครงสร้าง tree
      ถ้าเป็นไปได้ก็ควรเลี่ยง infinite loop และกรณีที่เลี่ยงไม่ได้จริง ๆ นั้นค่อนข้างหายาก
      ไม่ใช่การแก้ไข แต่เป็นกลยุทธ์บรรเทาปัญหาที่ดี
      infinite loop เป็นหนึ่งในบั๊กที่เลวร้ายที่สุด หาได้ง่ายใน debugger แต่ผลลัพธ์อาจไม่เป็นมิตรอย่างมาก เหมือนสถานการณ์ของ OP ที่ “แทบจะ ssh เข้าไปได้เท่านั้น”
      โดยเฉพาะ infinite loop ในโค้ดไลบรารี ยิ่งน่าหงุดหงิดกว่า
    • แบบนั้นดีกว่ามาก ใช้แค่ หน่วยความจำคงที่ ก็พอ
      มีการรับประกันว่าจำนวน node จะไม่เกินความสูงของ tree
  • exception ใน thread นั้นร้ายแรงจริง ๆ
    มีเรื่องเล่าการตามบั๊กสุดสยองที่มี C++, select(), และ thread ที่เหวี่ยง exception เป็นตัวเอก: https://news.ycombinator.com/item?id=42532979

    • จำได้ว่าเคยอ่านโพสต์นั้น แต่ความรู้ในสาขานั้นไม่พอเลยไม่เข้าใจ
      คงต้องกลับไปอ่านใหม่