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 1000 1001 1002 1003 1004 1005 1006 1007 1008 1009 1010 1011 1012 1013 1014 1015 1016 1017 1018 1019 1020 1021 1022 1023 1024 1025 1026 1027 1028 1029 1030 1031 1032 1033 1034 1035 1036 1037 1038 1039 1040 1041 1042 1043 1044 1045 1046
|
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<!-- Generated by Apache Maven Doxia Site Renderer 1.4 at 18 Feb 2015 -->
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
<title>CodeNarc - CodeNarc - Concurrency Rules</title>
<style type="text/css" media="all">
@import url("./css/maven-base.css");
@import url("./css/maven-theme.css");
@import url("./css/site.css");
</style>
<link rel="stylesheet" href="./css/print.css" type="text/css" media="print" />
<meta name="Date-Revision-yyyymmdd" content="20150218" />
<meta http-equiv="Content-Language" content="en" />
</head>
<body class="composite">
<div id="banner">
<a href="./" id="bannerLeft">
<img src="images/codenarc-logo.png" alt="CodeNarc" />
</a>
<a href="http://github.com/CodeNarc" id="bannerRight">
<img src="images/forkme_right_red_aa0000.png" alt="Fork me on GitHub" />
</a>
<div class="clear">
<hr/>
</div>
</div>
<div id="breadcrumbs">
<div class="xleft">
<span id="publishDate">Last Published: 18 Feb 2015</span>
| <span id="projectVersion">Version: 0.23</span>
</div>
<div class="xright">
</div>
<div class="clear">
<hr/>
</div>
</div>
<div id="leftColumn">
<div id="navcolumn">
<h5>General</h5>
<ul>
<li class="none">
<a href="index.html" title="Home">Home</a>
</li>
<li class="none">
<a href="https://sourceforge.net/project/showfiles.php?group_id=250145" class="externalLink" title="Downloads">Downloads</a>
</li>
<li class="none">
<a href="apidocs/index.html" title="Javadocs">Javadocs</a>
</li>
<li class="none">
<a href="http://sourceforge.net/mail/?group_id=250145" class="externalLink" title="Mailing Lists">Mailing Lists</a>
</li>
<li class="none">
<a href="http://sourceforge.net/tracker/?group_id=250145" class="externalLink" title="Bug Tracker">Bug Tracker</a>
</li>
<li class="none">
<a href="http://sourceforge.net/projects/codenarc" class="externalLink" title="SourceForge Project">SourceForge Project</a>
</li>
<li class="none">
<a href="http://github.com/CodeNarc" class="externalLink" title="GitHub Project">GitHub Project</a>
</li>
</ul>
<h5>Running</h5>
<ul>
<li class="none">
<a href="codenarc-ant-task.html" title="Ant Task Usage">Ant Task Usage</a>
</li>
<li class="none">
<a href="codenarc-command-line.html" title="Command-Line">Command-Line</a>
</li>
<li class="none">
<a href="codenarc-run-as-a-test.html" title="Run as a Test">Run as a Test</a>
</li>
<li class="none">
<a href="codenarc-other-tools-frameworks.html" title="Other Tools/Frameworks">Other Tools/Frameworks</a>
</li>
</ul>
<h5>Using</h5>
<ul>
<li class="none">
<a href="codenarc-creating-ruleset.html" title="Creating a RuleSet">Creating a RuleSet</a>
</li>
<li class="none">
<a href="codenarc-creating-rule.html" title="Creating a Rule">Creating a Rule</a>
</li>
<li class="none">
<a href="codenarc-configuring-rules.html" title="Configuring Rules">Configuring Rules</a>
</li>
<li class="none">
<a href="StarterRuleSet-AllRulesByCategory.groovy.txt" title="Starter RuleSet (All)">Starter RuleSet (All)</a>
</li>
</ul>
<h5>Report Types</h5>
<ul>
<li class="none">
<a href="codenarc-HtmlReportWriter.html" title="HTML Report">HTML Report</a>
</li>
<li class="none">
<a href="codenarc-XmlReportWriter.html" title="XML Report">XML Report</a>
</li>
<li class="none">
<a href="codenarc-TextReportWriter.html" title="Text and IDE Reports">Text and IDE Reports</a>
</li>
</ul>
<h5>Sample Reports</h5>
<ul>
<li class="none">
<a href="SampleCodeNarcHtmlReport.html" title="Sample HTML Report">Sample HTML Report</a>
</li>
<li class="none">
<a href="SampleCodeNarcXmlReport.xml" title="Sample XML Report">Sample XML Report</a>
</li>
</ul>
<h5>Rules</h5>
<ul>
<li class="none">
<a href="codenarc-rule-index.html" title="Rule Index">Rule Index</a>
</li>
<li class="none">
<a href="codenarc-rules-basic.html" title="Basic Rules">Basic Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-braces.html" title="Braces Rules">Braces Rules</a>
</li>
<li class="none">
<strong>Concurrency Rules</strong>
</li>
<li class="none">
<a href="codenarc-rules-convention.html" title="Convention Rules">Convention Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-design.html" title="Design Rules">Design Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-dry.html" title="DRY Rules">DRY Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-enhanced.html" title="Enhanced Rules">Enhanced Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-exceptions.html" title="Exceptions Rules">Exceptions Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-formatting.html" title="Formatting Rules">Formatting Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-generic.html" title="Generic Rules">Generic Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-grails.html" title="Grails Rules">Grails Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-groovyism.html" title="Groovyism Rules">Groovyism Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-imports.html" title="Imports Rules">Imports Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-jdbc.html" title="JDBC Rules">JDBC Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-junit.html" title="JUnit Rules">JUnit Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-logging.html" title="Logging Rules">Logging Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-naming.html" title="Naming Rules">Naming Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-size.html" title="Size/Complexity Rules">Size/Complexity Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-security.html" title="Security Rules">Security Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-serialization.html" title="Serialization Rules">Serialization Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-unnecessary.html" title="Unnecessary Rules">Unnecessary Rules</a>
</li>
<li class="none">
<a href="codenarc-rules-unused.html" title="Unused Rules">Unused Rules</a>
</li>
</ul>
<h5>Developing</h5>
<ul>
<li class="none">
<a href="codenarc-developer-guide.html" title="Developer Guide">Developer Guide</a>
</li>
</ul>
<h5>Project Documentation</h5>
<ul>
<li class="collapsed">
<a href="project-info.html" title="Project Information">Project Information</a>
</li>
<li class="collapsed">
<a href="project-reports.html" title="Project Reports">Project Reports</a>
</li>
</ul>
<a href="http://sourceforge.net" title="Hosted on SourceForge.net" class="poweredBy">
<img class="poweredBy" alt="Hosted on SourceForge.net" src="http://sflogo.sourceforge.net/sflogo.php?group_id=208647&type=2" />
</a>
<a href="http://maven.apache.org" title="Build with Maven 2" class="poweredBy">
<img class="poweredBy" alt="Build with Maven 2" src="images/logos/maven-feather.png" />
</a>
</div>
</div>
<div id="bodyColumn">
<div id="contentBox">
<div class="section">
<h2>Concurrency Rules ("<i>rulesets/concurrency.xml</i>")<a name="Concurrency_Rules_rulesetsconcurrency.xml"></a></h2><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<div class="section">
<h3><a name="BusyWait">BusyWait</a> Rule<a name="BusyWait_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p>Busy waiting (forcing a <tt>Thread.sleep()</tt> while waiting on a condition) should be avoided. Prefer using the gate and barrier objects in the <tt>java.util.concurrent</tt> package.</p>
<p>Example of violations:</p>
<div>
<pre> while (x) { Thread.sleep(1000) }
while (x) { Thread.sleep(1000) { /* interruption handler */} }
for (int x = 10; x; x--) {
sleep(1000) // sleep is added to Object in Groovy
}
// here is the proper way to wait:
countDownLatch.await()
// this is weird code to write, but does not cause a violation
for (def x : collections) {
sleep(1000)
}
while (x) {
// you should use a lock here, but technically you are
// not just busy waiting because you are doing other work
doSomething()
sleep(1000)
}
</pre></div></div>
<div class="section">
<h3><a name="DoubleCheckedLocking">DoubleCheckedLocking</a> Rule<a name="DoubleCheckedLocking_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p>This rule detects double checked locking, where a 'lock hint' is tested for null before initializing an object within a synchronized block. Double checked locking does not guarantee correctness and is an anti-pattern.</p>
<p>A full explanation of why double checked locking is broken in Java is available on Wikipedia: <a class="externalLink" href="http://en.wikipedia.org/wiki/Double-checked_locking"></a></p>
<p>Example of violations:</p>
<div>
<pre> if (object == null) {
synchronized(this) {
if (object == null) {
// createObject() could be called twice depending
// on the Thread Scheduler.
object = createObject()
}
}
}
// there are several idioms to fix this problem.
def result = object;
if (result == null) {
synchronized(this) {
result = object;
if (result == null)
object = result = createObject()
}
}
// and a better solution for a singleton:
class myClass {
private static class ObjectHolder {
public static Object object = createObject()
}
public static Object getObject() {
return ObjectHolder.object;
}
}
</pre></div></div>
<div class="section">
<h3><a name="InconsistentPropertyLocking">InconsistentPropertyLocking</a> Rule<a name="InconsistentPropertyLocking_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p>Class contains similarly-named get and set methods where one method of the pair is marked either @WithReadLock or @WithWriteLock and the other is not locked at all. This may result in incorrect behavior at runtime, as callers of the get and set methods will not necessarily lock correctly and my see an inconsistent state for the object. The get and set method should both be guarded by @WithReadLock/@WithWriteLock or neither should be guarded.</p>
<p>Example of violations:</p>
<div>
<pre> class Person {
String name
Date birthday
boolean deceased
boolean parent
@WithWriteLock setName(String name) {
this.name = name
}
// violation, get method should be locked
String getName() {
name
}
// violation, set method should be locked
void setBirthday(Date birthday) {
this.birthday = birthday
}
@WithReadLock String getBirthday() {
birthday
}
// violation, set method should be locked
void setDeceased(boolean deceased) {
this.deceased = deceased
}
@WithReadLock boolean isDeceased() {
deceased
}
@WithWriteLock void setParent(boolean parent) {
this.parent = parent
}
// violation, get method should be locked
boolean isParent() {
parent
}
}
</pre></div></div>
<div class="section">
<h3><a name="InconsistentPropertySynchronization">InconsistentPropertySynchronization</a> Rule<a name="InconsistentPropertySynchronization_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p>Class contains similarly-named get and set methods where the set method is synchronized and the get method is not, or the get method is synchronized and the set method is not. This may result in incorrect behavior at runtime, as callers of the get and set methods will not necessarily see a consistent state for the object. The get and set method should both be synchronized or neither should be synchronized.</p>
<p>Example of violations:</p>
<div>
<pre> class Person {
String name
Date birthday
boolean deceased
boolean parent
int weight
synchronized setName(String name) {
this.name = name
}
// violation, get method should be synchronized
String getName() {
name
}
// violation, set method should be synchronized
void setBirthday(Date birthday) {
this.birthday = birthday
}
synchronized String getBirthday() {
birthday
}
// violation, set method should be synchronized
void setDeceased(boolean deceased) {
this.deceased = deceased
}
synchronized boolean isDeceased() {
deceased
}
synchronized void setParent(boolean parent) {
this.parent = parent
}
// violation, get method should be synchronized
boolean isParent() {
parent
}
// violation get method should be synchronized
@groovy.transform.Synchronized
void setWeight(int value) {
weight = value
}
}
</pre></div></div>
<div class="section">
<h3><a name="NestedSynchronization">NestedSynchronization</a> Rule<a name="NestedSynchronization_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p>This rule reports occurrences of nested <tt>synchronized</tt> statements.</p>
<p>Nested <tt>synchronized</tt> statements should be avoided. Nested <tt>synchronized</tt> statements are either useless (if the lock objects are identical) or prone to deadlock.</p>
<p>Note that a <i>closure</i> or an <i>anonymous inner class</i> carries its own context (scope). A <tt>synchronized</tt> statement within a <i>closure</i> or an <i>anonymous inner class</i> defined within an outer <tt>synchronized</tt> statement does not cause a violation (though nested <tt>synchronized</tt> statements within either of those will).</p>
<p>Here is an example of code that produces a violation:</p>
<div>
<pre> def myMethod() {
synchronized(this) {
// do something ...
synchronized(this) {
// do something else ...
}
}
}
</pre></div></div>
<div class="section">
<h3><a name="StaticCalendarField">StaticCalendarField</a> Rule<a name="StaticCalendarField_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p><tt>Calendar</tt> objects should not be used as <tt>static</tt> fields. Calendars are inherently unsafe for multithreaded use. Sharing a single instance across thread boundaries without proper synchronization will result in erratic behavior of the application. Under 1.4 problems seem to surface less often than under Java 5 where you will probably see random <tt>ArrayIndexOutOfBoundsException</tt> or <tt>IndexOutOfBoundsException</tt> in <tt>sun.util.calendar.BaseCalendar.getCalendarDateFromFixedDate()</tt>. You may also experience serialization problems. Using an instance field or a <tt>ThreadLocal</tt> is recommended.</p>
<p>For more information on this see Sun Bug #6231579 and Sun Bug #6178997.</p>
<p>Examples:</p>
<div>
<pre> // Violations
class MyClass {
static Calendar calendar1
static java.util.Calendar calendar2
static final CAL1 = Calendar.getInstance()
static final CAL2 = Calendar.getInstance(Locale.FRANCE)
static def cal3 = Calendar.getInstance(timezone)
static Object cal4 = Calendar.getInstance(timezone, locale)
}
// These usages are OK
class MyCorrectClass {
private final Calendar calendar1
static ThreadLocal<Calendar> calendar2
}
</pre></div></div>
<div class="section">
<h3><a name="StaticConnection">StaticConnection</a> Rule<a name="StaticConnection_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.14</i></p>
<p>Creates violations when a <tt>java.sql.Connection</tt> object is used as a <tt>static</tt> field. Database connections stored in <tt>static</tt> fields will be shared between threads, which is unsafe and can lead to race conditions.</p>
<p>A transactional resource object such as database connection can only be associated with one transaction at a time. For this reason, a connection should not be shared between threads and should not be stored in a static field. See Section 4.2.3 of the <i>J2EE Specification</i> for more details.</p>
<p>References:</p>
<ul>
<li>Standards Mapping - Security Technical Implementation Guide Version 3 - (STIG 3) APP3630.1 CAT II</li>
<li>Standards Mapping - Common Weakness Enumeration - (CWE) CWE ID 362, CWE ID 567</li>
<li>Standards Mapping - SANS Top 25 2009 - (SANS 2009) Insecure Interaction - CWE ID 362</li>
<li>Standards Mapping - SANS Top 25 2010 - (SANS 2010) Insecure Interaction - CWE ID 362</li>
<li>Java 2 Platform Enterprise Edition Specification, v1.4 Sun Microsystems</li></ul></div>
<div class="section">
<h3><a name="StaticDateFormatField">StaticDateFormatField</a> Rule<a name="StaticDateFormatField_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p><tt>DateFormat</tt> objects should not be used as <tt>static</tt> fields. DateFormats are inherently unsafe for multithreaded use. Sharing a single instance across thread boundaries without proper synchronization will result in erratic behavior of the application. Under 1.4 problems seem to surface less often than under Java 5 where you will probably see random <tt>ArrayIndexOutOfBoundsException</tt> or <tt>IndexOutOfBoundsException</tt> in <tt>sun.util.calendar.BaseCalendar.getCalendarDateFromFixedDate()</tt>. You may also experience serialization problems. Using an instance field or a <tt>ThreadLocal</tt> is recommended.</p>
<p>For more information on this see Sun Bug #6231579 and Sun Bug #6178997.</p>
<p>Examples:</p>
<div>
<pre> // Violations
class MyClass {
static DateFormat dateFormat1
static java.text.DateFormat dateFormat2
static final DATE1 = DateFormat.getDateInstance(DateFormat.LONG, Locale.FRANCE)
static final def DATE2 = DateFormat.getDateInstance(DateFormat.LONG)
static Object date3 = DateFormat.getDateInstance()
static final DATETIME1 = DateFormat.getDateTimeInstance(DateFormat.LONG, DateFormat.SHORT, Locale.FRANCE)
static final def DATETIME2 = DateFormat.getDateTimeInstance(DateFormat.LONG, DateFormat.SHORT)
static final Object DATETIME3 = DateFormat.getDateTimeInstance()
static final TIME1 = DateFormat.getTimeInstance(DateFormat.LONG, Locale.FRANCE)
static final def TIME2 = DateFormat.getTimeInstance(DateFormat.LONG)
static final Object TIME3 = DateFormat.getTimeInstance()
}
// These usages are OK
class MyCorrectClass {
private DateFormat calendar1
static ThreadLocal<DateFormat> calendar2
}
</pre></div></div>
<div class="section">
<h3><a name="StaticMatcherField">StaticMatcherField</a> Rule<a name="StaticMatcherField_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p>Matcher objects should not be used as static fields. Calendars are inherently unsafe for multithreaded use. Sharing a single instance across thread boundaries without proper synchronization will result in erratic behavior of the application.</p>
<p>Example of violations:</p>
<div>
<pre> // two violations
class MyClass {
static Matcher matcher1
static java.util.regex.Matcher matcher2
}
// these usages are OK
class MyCorrectClass {
private Matcher matcher1
static ThreadLocal<Matcher> matcher2
}
</pre></div></div>
<div class="section">
<h3><a name="StaticSimpleDateFormatField">StaticSimpleDateFormatField</a> Rule<a name="StaticSimpleDateFormatField_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.14</i></p>
<p><tt>SimpleDateFormat</tt> objects should not be used as <tt>static</tt> fields. SimpleDateFormats are inherently unsafe for multithreaded use. Sharing a single instance across thread boundaries without proper synchronization will result in erratic behavior of the application. Under 1.4 problems seem to surface less often than under Java 5 where you will probably see random <tt>ArrayIndexOutOfBoundsException</tt> or <tt>IndexOutOfBoundsException</tt> in <tt>sun.util.calendar.BaseCalendar.getCalendarDateFromFixedDate()</tt>. You may also experience serialization problems. Using an instance field or a <tt>ThreadLocal</tt> is recommended.</p>
<p>For more information on this see Sun Bug #6231579 and Sun Bug #6178997.</p>
<p>Examples:</p>
<div>
<pre> // Violations
class MyClass {
static SimpleDateFormat dateFormat1
static java.text.SimpleDateFormat dateFormat2
static final DATE1 = new SimpleDateFormat()
static final DATE2 = new SimpleDateFormat('MM/dd')
static final DATE3 = new SimpleDateFormat('MM/dd', DateFormatSymbols.instance)
static date4 = new SimpleDateFormat('MM/dd', Locale.FRANCE)
static date5 = new java.text.SimpleDateFormat('MM/dd')
}
// These usages are OK
class MyCorrectClass {
private SimpleDateFormat calendar1
static ThreadLocal<SimpleDateFormat> calendar2
}
</pre></div></div>
<div class="section">
<h3><a name="SynchronizedMethod">SynchronizedMethod</a> Rule<a name="SynchronizedMethod_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p>This rule reports uses of the <tt>synchronized</tt> keyword on methods. Synchronized methods are the same as synchronizing on 'this', which effectively make your synchronization policy public and modifiable by other objects. To avoid possibilities of deadlock, it is better to synchronize on internal objects.</p>
<p>Here is an example of code that produces a violation:</p>
<div>
<pre> synchronized def myMethod() {
// do stuff ...
}
</pre></div></div>
<div class="section">
<h3><a name="SynchronizedOnGetClass">SynchronizedOnGetClass</a> Rule<a name="SynchronizedOnGetClass_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>Since CodeNarc 0.11</i></p>
<p>Checks for synchronization on <tt>getClass()</tt> rather than class literal. This instance method synchronizes on <tt>this.getClass()</tt>. If this class is subclassed, subclasses will synchronize on the class object for the subclass, which isn't likely what was intended.</p></div>
<div class="section">
<h3><a name="SynchronizedOnBoxedPrimitive">SynchronizedOnBoxedPrimitive</a> Rule<a name="SynchronizedOnBoxedPrimitive_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p>The code synchronizes on a boxed primitive constant, such as an Integer. Since Integer objects can be cached and shared, this code could be synchronizing on the same object as other, unrelated code, leading to unresponsiveness and possible deadlock.</p>
<p>Example of violations:</p>
<div>
<pre> class MyClass {
Byte byte1 = 100
Short short1 = 1
Double double1 = 1
Integer integer1 = 1
Long long1 = 1
Float float1 = 1
Character char1 = 1
byte byte2 = getValue()
short short2 = getValue()
double double2 = getValue()
int integer2 = getValue()
long long2 = getValue()
float float2 = getValue()
char char2 = getValue()
def byte3 = new Byte((byte)100)
def short3 = new Short((short)1)
def double3 = new Double((double)1)
def integer3 = new Integer(1)
def long3 = new Long(1)
def float3 = new Float(1)
def char3 = new Character((char)'1')
def byte4 = 1 as byte
def short4 = 1 as short
def double4 = 1 as double
def integer4 = 1 as int
def long4 = 1 as long
def float4 = 1 as float
def char4 = 1 as char
def byte5 = 1 as Byte
def short5 = 1 as Short
def double5 = 1 as Double
def integer5 = 1 as Integer
def long5 = 1 as Long
def float5 = 1 as Float
def char5 = 1 as Character
def byte6 = (byte)1
def short6 = (short)1
def double6 = (double)1
def integer6 = (int)1
def long6 = (long)1
def float6 = (float)1
def char6 = (char)1
def method() {
// all of these synchronization blocks produce violations
synchronized(byte1) {}
synchronized(short1) {}
synchronized(double1) {}
synchronized(integer1) {}
synchronized(long1) {}
synchronized(float1) {}
synchronized(char1) {}
synchronized(byte2) {}
synchronized(short2) {}
synchronized(double2) {}
synchronized(integer2) {}
synchronized(long2) {}
synchronized(float2) {}
synchronized(char2) {}
synchronized(byte3) {}
synchronized(short3) {}
synchronized(double3) {}
synchronized(integer3) {}
synchronized(long3) {}
synchronized(float3) {}
synchronized(char3) {}
synchronized(byte4) {}
synchronized(short4) {}
synchronized(double4) {}
synchronized(integer4) {}
synchronized(long4) {}
synchronized(float4) {}
synchronized(char4) {}
synchronized(byte5) {}
synchronized(short5) {}
synchronized(double5) {}
synchronized(integer5) {}
synchronized(long5) {}
synchronized(float5) {}
synchronized(char5) {}
synchronized(byte6) {}
synchronized(short6) {}
synchronized(double6) {}
synchronized(integer6) {}
synchronized(long6) {}
synchronized(float6) {}
synchronized(char6) {}
}
}
</pre></div>
<p>And here is an in-depth example of how it works within inner classes and such:</p>
<div>
<pre> class MyClass {
final String lock = false
def method() {
// violation
synchronized(lock) { }
}
}
class MyClass {
final String lock = false
class MyInnerClass {
def method() {
// violation
synchronized(lock) { }
}
}
}
class MyClass {
// implicit typing
final def lock = true
def method() {
// violation
synchronized(lock) { }
}
}
class MyClass {
// implicit typing
final def lock = new Object[0] // correct idiom
def method() {
return new Runnable() {
final def lock = false // shadows parent from inner class
public void run() {
// violation
synchronized(stringLock) { }
}
}
}
}
class MyClass {
// implicit typing
final def lock = new Object[0] // correct idiom
class MyInnerClass {
final def lock = true // shadows parent from inner class
def method() {
// violation
synchronized(stringLock) { }
}
}
}
</pre></div></div>
<div class="section">
<h3><a name="SynchronizedOnString">SynchronizedOnString</a> Rule<a name="SynchronizedOnString_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p>Synchronization on a String field can lead to deadlock. Constant Strings are interned and shared across all other classes loaded by the JVM. Thus, this could is locking on something that other code might also be locking. This could result in very strange and hard to diagnose blocking and deadlock behavior.</p>
<p>See <a class="externalLink" href="http://www.javalobby.org/java/forums/t96352.html and http://jira.codehaus.org/browse/JETTY-352">JETTY-352</a>.</p>
<p>Examples:</p>
<div>
<pre> class MyClass {
final String stringLock = "stringLock"
def method() {
// violation
synchronized(stringLock) { }
}
}
class MyClass {
final String stringLock = "stringLock"
class MyInnerClass {
def method() {
synchronized(stringLock) { }
}
}
}
class MyClass {
// implicit typing
final def stringLock = "stringLock"
def method() {
// violation
synchronized(stringLock) { }
}
}
class MyClass {
// implicit typing
final def lock = new Object[0] // correct idiom
def method() {
return new Runnable() {
final def lock = "" // shadows parent from inner class
public void run() {
// violation
synchronized(stringLock) { }
}
}
}
}
class MyClass {
// implicit typing
final def lock = new Object[0] // correct idiom
class MyInnerClass {
final def lock = "" // shadows parent from inner class
def method() {
// violation
synchronized(stringLock) { }
}
}
}
</pre></div></div>
<div class="section">
<h3><a name="SynchronizedOnThis">SynchronizedOnThis</a> Rule<a name="SynchronizedOnThis_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p>This rule reports uses of the <tt>synchronized</tt> blocks where the synchronization reference is 'this'. Doing this effectively makes your synchronization policy public and modifiable by other objects. To avoid possibilities of deadlock, it is better to synchronize on internal objects.</p>
<p>Here is an example of code that produces a violation:</p>
<div>
<pre> def method3() {
synchronized(this) {
// do stuff ...
}
}
</pre></div></div>
<div class="section">
<h3><a name="SynchronizedReadObjectMethod">SynchronizedReadObjectMethod</a> Rule<a name="SynchronizedReadObjectMethod_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p>Catches Serializable classes that define a synchronized readObject method. By definition, an object created by deserialization is only reachable by one thread, and thus there is no need for readObject() to be synchronized. If the readObject() method itself is causing the object to become visible to another thread, that is an example of very dubious coding style.</p>
<p>Examples:</p>
<div>
<pre> class MyClass implements Serializable {
private synchronized void readObject(ObjectInputStream input) throws IOException, ClassNotFoundException {
// violation, no need to synchronized
}
}
class MyClass implements Serializable {
private void readObject(ObjectInputStream input) throws IOException, ClassNotFoundException {
synchronized(lock) {
// violation, no need to synchronized
}
}
}
// OK, class not Serializable
class MyClass {
private synchronized void readObject(ObjectInputStream input) throws IOException, ClassNotFoundException { }
}
// OK, class not Serializable
class MyClass {
private void readObject(ObjectInputStream input) throws IOException, ClassNotFoundException {
synchronized(lock) { }
}
}
class MyClass implements Serializable {
private void readObject(ObjectInputStream input) throws IOException, ClassNotFoundException {
// OK, this block is more than just a simple sync statement
synchronized(lock) { }
doSomething()
}
}
</pre></div></div>
<div class="section">
<h3><a name="SynchronizedOnReentrantLock">SynchronizedOnReentrantLock</a> Rule<a name="SynchronizedOnReentrantLock_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p>Synchronizing on a ReentrantLock field is almost never the intended usage. A ReentrantLock should be obtained using the lock() method and released in a finally block using the unlock() method.</p>
<p>This rule take from Alex Miller's <a class="externalLink" href="http://www.slideshare.net/alexmiller/java-concurrency-gotchas-3666977">Java Concurrency in Practice</a> slides.</p>
<p>Here is the proper usage of ReentrantLock:</p>
<div>
<pre> import java.util.concurrent.locks.ReentrantLock;
final lock = new ReentrantLock();
def method() {
//Trying to enter the critical section
lock.lock(); // will wait until this thread gets the lock
try {
// critical section
} finally {
//releasing the lock so that other threads can get notifies
lock.unlock();
}
}
</pre></div>
<p>Example of violations:</p>
<div>
<pre> class MyClass {
final ReentrantLock lock = new ReentrantLock()
def method() {
// violation
synchronized(lock) { }
}
}
class MyClass {
final ReentrantLock lock = new ReentrantLock()
class MyInnerClass {
def method() {
synchronized(lock) { }
}
}
}
class MyClass {
// implicit typing
final def lock = new ReentrantLock()
def method() {
// violation
synchronized(lock) { }
}
}
class MyClass {
// implicit typing
final def lock = new Object[0] // correct idiom
def method() {
return new Runnable() {
final def lock = new ReentrantLock() // shadows parent from inner class
public void run() {
// violation
synchronized(lock) { }
}
}
}
}
class MyClass {
// implicit typing
final def lock = new Object[0] // correct idiom
class MyInnerClass {
final def lock = new ReentrantLock() // shadows parent from inner class
def method() {
// violation
synchronized(lock) { }
}
}
}
</pre></div></div>
<div class="section">
<h3><a name="SystemRunFinalizersOnExit">SystemRunFinalizersOnExit</a> Rule<a name="SystemRunFinalizersOnExit_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p>This rule reports uses of the <tt>System.runFinalizersOnExit()</tt> method.</p>
<p>Method calls to <tt>System.runFinalizersOnExit()</tt> should not be allowed. This method is inherently non-thread-safe, may result in data corruption, deadlock, and may affect parts of the program far removed from it's call point. It is deprecated, and it's use strongly discouraged.</p>
<p>Here is an example of code that produces a violation:</p>
<div>
<pre> def method() {
System.runFinalizersOnExit(true)
}
</pre></div></div>
<div class="section">
<h3><a name="ThisReferenceEscapesConstructor">ThisReferenceEscapesConstructor</a> Rule<a name="ThisReferenceEscapesConstructor_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>Since CodeNarc 0.19</i></p>
<p>Reports constructors passing the 'this' reference to other methods. This equals exposing a half-baked objects and can lead to race conditions during initialization. For reference, see <a class="externalLink" href="http://www.slideshare.net/alexmiller/java-concurrency-gotchas-3666977/38">Java Concurrency in Practice</a> by Alex Miller and <a class="externalLink" href="http://www.ibm.com/developerworks/java/library/j-jtp0618/index.html">Java theory and practice: Safe construction techniques</a> by Brian Goetz.</p>
<p>Example of violations:</p>
<div>
<pre> class EventListener {
EventListener(EventPublisher publisher) {
publisher.register(this)
new WorkThread(publisher, this).start()
new AnotherWorkThread(listener: this)
}
}
</pre></div></div>
<div class="section">
<h3><a name="ThreadGroup">ThreadGroup</a> Rule<a name="ThreadGroup_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p>Avoid using <tt>ThreadGroup</tt>; although it is intended to be used in a threaded environment it contains methods that are not thread safe.</p>
<p>Here is an example of code that produces a violation:</p>
<div>
<pre> new ThreadGroup("...")
new ThreadGroup(tg, "my thread group")
Thread.currentThread().getThreadGroup()
System.getSecurityManager().getThreadGroup()
</pre></div></div>
<div class="section">
<h3><a name="ThreadLocalNotStaticFinal">ThreadLocalNotStaticFinal</a> Rule<a name="ThreadLocalNotStaticFinal_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p>This rule reports definition of the <tt>ThreadLocal</tt> fields that are not <tt>static</tt> and <tt>final</tt>.</p>
<p><i>ThreadLocal</i> fields should be <tt>static</tt> and <tt>final</tt>. In the most common case a <tt>java.lang.ThreadLocal</tt> instance associates state with a thread. A non-<tt>static</tt> non-<tt>final</tt> <tt>java.lang.ThreadLocal</tt> field associates state with an instance-thread combination. This is seldom necessary and often a bug which can cause memory leaks and possibly incorrect behavior.</p>
<p>Here is an example of code that produces a violation:</p>
<div>
<pre> private static ThreadLocal local1 = new ThreadLocal()
private final ThreadLocal local2 = new ThreadLocal()
protected ThreadLocal local3 = new ThreadLocal()
ThreadLocal local4 = new ThreadLocal()
</pre></div></div>
<div class="section">
<h3><a name="ThreadYield">ThreadYield</a> Rule<a name="ThreadYield_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~ -->
<p>This rule reports uses of the <tt>Thread.yield()</tt> method.</p>
<p>Method calls to <tt>Thread.yield()</tt> should not be allowed. This method has no useful guaranteed semantics, and is often used by inexperienced programmers to mask race conditions.</p>
<p>Here is an example of code that produces a violation:</p>
<div>
<pre> def method() {
Thread.yield()
}
</pre></div></div>
<div class="section">
<h3><a name="UseOfNotifyMethod">UseOfNotifyMethod</a> Rule<a name="UseOfNotifyMethod_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>Since CodeNarc 0.11</i></p>
<p>Checks for code that calls <tt>notify()</tt> rather than <tt>notifyAll()</tt>. Java monitors are often used for multiple conditions. Calling <tt>notify()</tt> only wakes up one thread, meaning that the awakened thread might not be the one waiting for the condition that the caller just satisfied.</p>
<p>Also see <a class="externalLink" href="http://www.javaconcurrencyinpractice.com/"><b>Java_Concurrency_in_Practice</b></a>, Brian Goetz, p 303.</p></div>
<div class="section">
<h3><a name="VolatileArrayField">VolatileArrayField</a> Rule<a name="VolatileArrayField_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>New in CodeNarc 0.13</i></p>
<p>Volatile array fields are unsafe because the contents of the array are not treated as volatile. Changing the entire array reference is visible to other threads, but changing an array element is not.</p>
<p>This rule take from Alex Miller's <i>Java Concurrency in Practice</i> slides, available at <a class="externalLink" href="http://www.slideshare.net/alexmiller/java-concurrency-gotchas-3666977">http://www.slideshare.net/alexmiller/java-concurrency-gotchas-3666977</a>.</p>
<p>Example of violations:</p>
<div>
<pre> class MyClass {
private volatile Object[] field1 = value()
volatile field2 = value as Object[]
volatile field3 = (Object[])foo
}
</pre></div></div>
<div class="section">
<h3><a name="VolatileLongOrDoubleField">VolatileLongOrDoubleField</a> Rule<a name="VolatileLongOrDoubleField_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p>This rule reports on <tt>long</tt> or <tt>double</tt> fields that are declared <tt>volatile</tt>.</p>
<p>Long or double fields should not be declared as <tt>volatile</tt>. Java specifies that reads and writes from such fields are atomic, but many JVM's have violated this specification. Unless you are certain of your JVM, it is better to synchronize access to such fields rather than declare them <tt>volatile</tt>. This rule flags fields marked <tt>volatile</tt> when their type is <tt>double</tt> or <tt>long</tt> or the name of their type is "Double" or "Long".</p>
<p>Here is an example of code that produces a violation:</p>
<div>
<pre> def method() {
private volatile double d
private volatile long f
}
</pre></div></div>
<div class="section">
<h3><a name="WaitOutsideOfWhileLoop">WaitOutsideOfWhileLoop</a> Rule<a name="WaitOutsideOfWhileLoop_Rule"></a></h3><!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
<p><i>Since CodeNarc 0.13</i></p>
<p>Calls to <tt>Object.wait()</tt> must be within a <tt>while</tt> loop. This ensures that the awaited condition has not already been satisfied by another thread before the <tt>wait()</tt> is invoked. It also ensures that the proper thread was resumed and guards against incorrect notification. See [1] and [3].</p>
<p>As a more modern and flexible alternative, consider using the Java <i>concurrency utilities</i> instead of <tt>wait()</tt> and <tt>notify()</tt>. See discussion in <i>Effective Java</i> [2].</p>
<p>Example of violation:</p>
<div>
<pre> class MyClass {
private data
void processData()
synchronized(data) {
if (!data.isReady()) {
data.wait()
}
data.calculateStatistics()
}
}
}
</pre></div>
<p>Example of correct usage:</p>
<div>
<pre> class MyClass {
private data
void processData()
synchronized(data) {
while (!data.isReady()) {
data.wait()
}
data.calculateStatistics()
}
}
}
</pre></div>
<div class="section">
<h4>References<a name="References"></a></h4>
<ul>
<li>[1] <b>Effective Java, Programming Language Guide</b>, by Joshua Bloch. Addison Wesley (2001). Chapter 50 (1st edition) is entitled "Never invoke wait outside a loop."</li>
<li>[2] <b>Effective Java</b>, 2nd edition, by Joshua Bloch, Addison Wesley (2008). Item #69: <i>Prefer concurrency utilities to wait and notify</i>.</li>
<li>[3] Software Engineering Institute - Secure Coding <a class="externalLink" href="https://www.securecoding.cert.org/confluence/display/java/THI03-J.+Always+invoke+wait()+and+await()+methods+inside+a+loop">discussion of this issue</a></li></ul></div></div></div>
</div>
</div>
<div class="clear">
<hr/>
</div>
<div id="footer">
<div class="xright">
Copyright © 2015.
All Rights Reserved.
</div>
<div class="clear">
<hr/>
</div>
</div>
</body>
</html>
|