[Java Client] Collection 객체, Map 객체의 동시성 제어 문제
문제점
https://github.com/naver/arcus-java-client/pull/769/commits/3d216eee6423aa14bed9bd19d1e6e4281300089a
위 PR을 리뷰하며 Collections.synchronizedList() 사용을 고려했습니다.
그러나 아래와 같은 자료를 찾아, Collections.synchronizedList()는 사실 동시성 제어를 보장하지 않는다는 것을 알게 되었습니다.
https://stackoverflow.com/a/28998561
이것이 사실인지 검증하기 위해 아래와 같은 테스트 코드를 수행했습니다.
@Test
public void testConcurrency() throws InterruptedException {
final List<String> l = Collections.synchronizedList(new ArrayList<>());
final int count = 1000;
Thread t = new Thread(() -> {
while (true) {
for (String e : l) {
System.out.println(e);
}
if (l.size() == count) {
System.out.println(l);
break;
}
}
});
t.start();
for (int i = 0; i < count; i++) {
l.add(String.valueOf(i));
}
t.join();
}
결과는 다음과 같이 ConcurrentModificationException이 발생합니다.
Exception in thread "Thread-0" java.util.ConcurrentModificationException
at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1013)
at java.base/java.util.ArrayList$Itr.next(ArrayList.java:967)
at net.spy.memcached.MultibyteKeyTest.lambda$testConcurrency$0(MultibyteKeyTest.java:118)
at java.base/java.lang.Thread.run(Thread.java:840)
다른 부분은 다 동일하고, List 객체를 CopyOnWriteArrayList 객체로 변경했을 때는 다음과 같이 의도된 출력 결과가 나옵니다.
0
1
2
... // 중략
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163, 164, 165, 166, 167, 168, 169, 170, 171, 172, 173, 174, 175, 176, 177, 178, 179, 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, 190, 191, 192, 193, 194, 195, 196, 197, 198, 199, 200, 201, 202, 203, 204, 205, 206, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 220, 221, 222, 223, 224, 225, 226, 227, 228, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254, 255, 256, 257, 258, 259, 260, 261, 262, 263, 264, 265, 266, 267, 268, 269, 270, 271, 272, 273, 274, 275, 276, 277, 278, 279, 280, 281, 282, 283, 284, 285, 286, 287, 288, 289, 290, 291, 292, 293, 294, 295, 296, 297, 298, 299, 300, 301, 302, 303, 304, 305, 306, 307, 308, 309, 310, 311, 312, 313, 314, 315, 316, 317, 318, 319, 320, 321, 322, 323, 324, 325, 326, 327, 328, 329, 330, 331, 332, 333, 334, 335, 336, 337, 338, 339, 340, 341, 342, 343, 344, 345, 346, 347, 348, 349, 350, 351, 352, 353, 354, 355, 356, 357, 358, 359, 360, 361, 362, 363, 364, 365, 366, 367, 368, 369, 370, 371, 372, 373, 374, 375, 376, 377, 378, 379, 380, 381, 382, 383, 384, 385, 386, 387, 388, 389, 390, 391, 392, 393, 394, 395, 396, 397, 398, 399, 400, 401, 402, 403, 404, 405, 406, 407, 408, 409, 410, 411, 412, 413, 414, 415, 416, 417, 418, 419, 420, 421, 422, 423, 424, 425, 426, 427, 428, 429, 430, 431, 432, 433, 434, 435, 436, 437, 438, 439, 440, 441, 442, 443, 444, 445, 446, 447, 448, 449, 450, 451, 452, 453, 454, 455, 456, 457, 458, 459, 460, 461, 462, 463, 464, 465, 466, 467, 468, 469, 470, 471, 472, 473, 474, 475, 476, 477, 478, 479, 480, 481, 482, 483, 484, 485, 486, 487, 488, 489, 490, 491, 492, 493, 494, 495, 496, 497, 498, 499, 500, 501, 502, 503, 504, 505, 506, 507, 508, 509, 510, 511, 512, 513, 514, 515, 516, 517, 518, 519, 520, 521, 522, 523, 524, 525, 526, 527, 528, 529, 530, 531, 532, 533, 534, 535, 536, 537, 538, 539, 540, 541, 542, 543, 544, 545, 546, 547, 548, 549, 550, 551, 552, 553, 554, 555, 556, 557, 558, 559, 560, 561, 562, 563, 564, 565, 566, 567, 568, 569, 570, 571, 572, 573, 574, 575, 576, 577, 578, 579, 580, 581, 582, 583, 584, 585, 586, 587, 588, 589, 590, 591, 592, 593, 594, 595, 596, 597, 598, 599, 600, 601, 602, 603, 604, 605, 606, 607, 608, 609, 610, 611, 612, 613, 614, 615, 616, 617, 618, 619, 620, 621, 622, 623, 624, 625, 626, 627, 628, 629, 630, 631, 632, 633, 634, 635, 636, 637, 638, 639, 640, 641, 642, 643, 644, 645, 646, 647, 648, 649, 650, 651, 652, 653, 654, 655, 656, 657, 658, 659, 660, 661, 662, 663, 664, 665, 666, 667, 668, 669, 670, 671, 672, 673, 674, 675, 676, 677, 678, 679, 680, 681, 682, 683, 684, 685, 686, 687, 688, 689, 690, 691, 692, 693, 694, 695, 696, 697, 698, 699, 700, 701, 702, 703, 704, 705, 706, 707, 708, 709, 710, 711, 712, 713, 714, 715, 716, 717, 718, 719, 720, 721, 722, 723, 724, 725, 726, 727, 728, 729, 730, 731, 732, 733, 734, 735, 736, 737, 738, 739, 740, 741, 742, 743, 744, 745, 746, 747, 748, 749, 750, 751, 752, 753, 754, 755, 756, 757, 758, 759, 760, 761, 762, 763, 764, 765, 766, 767, 768, 769, 770, 771, 772, 773, 774, 775, 776, 777, 778, 779, 780, 781, 782, 783, 784, 785, 786, 787, 788, 789, 790, 791, 792, 793, 794, 795, 796, 797, 798, 799, 800, 801, 802, 803, 804, 805, 806, 807, 808, 809, 810, 811, 812, 813, 814, 815, 816, 817, 818, 819, 820, 821, 822, 823, 824, 825, 826, 827, 828, 829, 830, 831, 832, 833, 834, 835, 836, 837, 838, 839, 840, 841, 842, 843, 844, 845, 846, 847, 848, 849, 850, 851, 852, 853, 854, 855, 856, 857, 858, 859, 860, 861, 862, 863, 864, 865, 866, 867, 868, 869, 870, 871, 872, 873, 874, 875, 876, 877, 878, 879, 880, 881, 882, 883, 884, 885, 886, 887, 888, 889, 890, 891, 892, 893, 894, 895, 896, 897, 898, 899, 900, 901, 902, 903, 904, 905, 906, 907, 908, 909, 910, 911, 912, 913, 914, 915, 916, 917, 918, 919, 920, 921, 922, 923, 924, 925, 926, 927, 928, 929, 930, 931, 932, 933, 934, 935, 936, 937, 938, 939, 940, 941, 942, 943, 944, 945, 946, 947, 948, 949, 950, 951, 952, 953, 954, 955, 956, 957, 958, 959, 960, 961, 962, 963, 964, 965, 966, 967, 968, 969, 970, 971, 972, 973, 974, 975, 976, 977, 978, 979, 980, 981, 982, 983, 984, 985, 986, 987, 988, 989, 990, 991, 992, 993, 994, 995, 996, 997, 998, 999]
Collections.synchronizedList() Java Doc에 있는 주석에 의하면, Iteration 시 아래와 같이 사용자가 직접 Lock을 잡아야 합니다.
List list = Collections. synchronizedList(new ArrayList());
...
synchronized (list) {
Iterator i = list. iterator(); // Must be in synchronized block
while (i. hasNext())
foo(i. next());
}
다음과 같이 Iteration 시 Lock을 잡도록 테스트 코드를 수정했을 때는 ConcurrentModificationException이 발생하지 않고 의도된 결과가 출력됩니다.
@Test
public void testConcurrency() throws InterruptedException {
final List<String> l = Collections.synchronizedList(new ArrayList<>());
final int count = 1000;
Thread t = new Thread(() -> {
while (true) {
synchronized (l) {
for (String e : l) {
System.out.println(e);
}
}
if (l.size() == count) {
System.out.println(l);
break;
}
}
});
t.start();
for (int i = 0; i < count; i++) {
l.add(String.valueOf(i));
}
t.join();
}
AS-IS
현재 Collections.synchronized...()을 사용하는 코드는 SMGetResult 클래스 뿐입니다.
public SMGetResult(int totalResultElementCount, boolean reverse) {
this.totalResultElementCount = totalResultElementCount;
this.reverse = reverse;
this.missedKeyList = Collections.synchronizedList(new ArrayList<>());
this.missedKeyMap
= Collections.synchronizedMap(new HashMap<>());
this.trimmedKeyMap = Collections.synchronizedMap(new HashMap<>());
this.mergedTrimmedKeys = new ArrayList<>();
this.mergedResult = new ArrayList<>(totalResultElementCount);
}
TO-BE
아래와 같이 정말 동시성 제어가 제공되는 객체를 사용해야 합니다.
public SMGetResult(int totalResultElementCount, boolean reverse) {
this.totalResultElementCount = totalResultElementCount;
this.reverse = reverse;
this.missedKeyList = new CopyOnWriteArrayList<>();
this.missedKeyMap = new ConcurrentHashMap<>();
this.trimmedKeyMap = new ConcurrentHashMap<>();
this.mergedTrimmedKeys = new ArrayList<>();
this.mergedResult = new ArrayList<>(totalResultElementCount);
}
Collections.synchronizedMap 대신에 ConcurrentHashMap 사용하면, ConcurrentModificationException 발생을 막을 수 있나요?
@jhpark816 현민님 대신 답변드리자면, 말씀하신대로 동작합니다.
@oliviarla @brido4125 TO-BE 자료구조로 변경하는 것이 좋은 지를 검토 바랍니다.
@jhpark816
제 생각엔 missedKeyList, missedKeyMap, trimmedKeyMap 등의 자료구조를 가져오기 전에 해당 연산의 getter 내에서 Future.get()을 호출하여서 명시적으로 연산이 완료된 후에 가져올 수 있음을 사용자에게 알려주는게 좋을 것 같습니다.
위와 같은 방법 사용 시, 별도의 동기화 자료구조를 사용하지 않아도 됩니다.
ConcurrentModificationException 발생하도록 두어도 될 것 같습니다. 의견으로 참고 바랍니다.
-
future.get()호출 이후에 missed or trimmed keys 조회한다면 해당 예외가 발생하지 않습니다. -
future.get()호출 없이 missed or trimmed keys 조회할 경우에는 예외가 발생할 수 있습니다. 이러한 예외가 발생하면 응용에 치명적일 수 있나요?
스프링 프레임워크 기반의 WAS 환경에서 ConcurrentModificationException이 발생했을 때 이에 대한 예외 처리를 해주지 않으면 사용자가 에러 응답을 받습니다. 따라서 ConcurrentModificationException이 발생할 수 있음을 응용 개발자가 사전에 인지하고 이에 대한 처리를 해주어야 합니다.
ConcurrentModificationException 발생하도록 둔다면 Collections.synchronized...()을 사용하지 않아도 되고, 따라서 일반적인 ArrayList와 HashMap을 사용하면 됩니다.
저도 ConcurrentModificationException을 막기 위해 future.get() 호출 등의 방법으로 연산이 모두 완료된 후에만 missed, trimmed keys를 조회할 수 있도록 하는 것이 좋은 것 같습니다.
@uhm0311
우선은 동기화 자료구조를 사용하면 좋겠습니다.
아래와 같이 수정할 수 있는 지 한번 검토해 주세요. missedKeyMap이 있어서 missedKeyList 항목은 제거할 수 있지 않나 생각됩니다.
public SMGetResult(int totalResultElementCount, boolean reverse) {
this.totalResultElementCount = totalResultElementCount;
this.reverse = reverse;
// this.missedKeyList = new CopyOnWriteArrayList<>();
this.missedKeyMap = new ConcurrentHashMap<>();
this.trimmedKeyMap = new ConcurrentHashMap<>();
this.mergedTrimmedKeys = new ArrayList<>();
this.mergedResult = new ArrayList<>(totalResultElementCount);
}
@jhpark816
missedKeyList 필드를 제거하고 Getter를 아래와 같이 수정할 수 있습니다.
public List<String> getMissedKeyList() {
return new ArrayList<>(missedKeyMap.keySet());
}
@uhm0311 동기화 자료 구조를 사용하면서, missedKeyList 필드를 제거하시죠.
위의 PR로 본 이슈는 처리가 완료되었으므로, close 합니다.