Branch: refs/heads/stable
  Home:   https://github.com/jenkinsci/jenkins
  Commit: 6b9c4ee6e8905a6eb701a95fcca63559e6bf2fa1
      
https://github.com/jenkinsci/jenkins/commit/6b9c4ee6e8905a6eb701a95fcca63559e6bf2fa1
  Author: Kohsuke Kawaguchi <[email protected]>
  Date:   2014-07-31 (Thu, 31 Jul 2014)

  Changed paths:
    M changelog.html
    M core/pom.xml

  Log Message:
  -----------
  [FIXED JENKINS-23410]

Integrated the fix made in winp 1.20.

(cherry picked from commit fad3f7db48e4184221469fbbb0edc2163e26ac0f)


  Commit: 58e6c7ba4d7fafc9dcfd349476876e2876b86d5b
      
https://github.com/jenkinsci/jenkins/commit/58e6c7ba4d7fafc9dcfd349476876e2876b86d5b
  Author: Daniel Beck <[email protected]>
  Date:   2014-07-31 (Thu, 31 Jul 2014)

  Changed paths:
    M core/src/main/java/hudson/model/Api.java

  Log Message:
  -----------
  [JENKINS-17374 JENKINS-18116] Don't set gzip header for error

(cherry picked from commit 0a241aafdbdf3ad15d34efa3077658c5770b9195)


  Commit: 2baa7caade6923b9a2023e64b60bb537fcc65077
      
https://github.com/jenkinsci/jenkins/commit/2baa7caade6923b9a2023e64b60bb537fcc65077
  Author: Kohsuke Kawaguchi <[email protected]>
  Date:   2014-08-01 (Fri, 01 Aug 2014)

  Changed paths:
    M test/src/test/java/jenkins/model/lazy/LazyBuildMixInTest.java

  Log Message:
  -----------
  [JENKINS-22395]

I'm not sure if I understand how the original fix in PR #1190 (f1430a2) 
addresses the problem.

My understanding is of the problem is as follows:

  - someone deletes 'b2' but holds on to its reference (could be different 
threads)
  - someone calls b2.getPreviousBuild()
    - if b2.previousBuildR is null, then this triggers the loading of b1, and
      that process sets up a bi-di link via b1<->b2 via b1.nextBuildR <-> 
b2.previousBuildR
    - this makes b1.getNextBuild() incorrectly return b2

Presumably f1430a2 addresses this somehow, but I think I can induce this 
situation in other ways,
which is what dropLinksAfterGC2() does.

In this test,

initial state:

   b1 <-> b2 <-> b3

   we load everyone and connect them all up

after deleting b2:

   b1 <--------> b3
    ^            ^
    +---- b2 ----+

  b1 and b3 points each other, and b2 still refers to each side

after dropping b1:
     b2 --> b3

now, here, when I do b2.getPreviousBuild(), it loads b1a and it sets 
b1a.nextBuildR to b2.

    b1a <-> b2 --> b3

So I claim this is a proof that the fix is incomplete, even for the problem 
JENKINS-22395 has discovered.

I don't think that the problem is for the dropLinks call to fail to patch up 
references.
The problem is that b2.getPreviousBuild() forcing b1 to point to b2, because if 
b2 is deleted and assumed to be
invalid, then no matter what bX this method will find you never want 
bX.nextBuildR to point to b2.

(cherry picked from commit 53e95ee517259e1e10d7209bcbaa4314397c1f40)


  Commit: 3f0388379b06b31ebec1bf17cc976001fcd80b7b
      
https://github.com/jenkinsci/jenkins/commit/3f0388379b06b31ebec1bf17cc976001fcd80b7b
  Author: Kohsuke Kawaguchi <[email protected]>
  Date:   2014-08-01 (Fri, 01 Aug 2014)

  Changed paths:
    M test/src/test/java/jenkins/model/lazy/LazyBuildMixInTest.java

  Log Message:
  -----------
  [JENKINS-22395] correcting the control test

As I step-executed the code, I discovered b2.getPreviousBuild() was getting 
invoked
between BRHF.drop(b1) and b2.dropLinks() call, in 
PeepholePermalink.RunListenerImpl.onDeleted()
because Run.delete() calls RunListener.fireDeleted(this).

Thus in effect the sequence of the call order was as follows:
   assertEquals(1, BRHF.drop(b1));
  b2.getPreviousBuild(); // happens indirectly in PeepholePermalink
  b2.delete();
  FreeStyleBuild b1a = b2.getPreviousBuild();

This defeats the purpose of the fix, because in this call sequence, by 
b2.dropLinks()
as implemented before f1430a2 will correctly fix up b1a.nextBuildR to b3.

For the test to work as intended, it is important that 
b2.previousBuildR.get()==null
during dropLinks(). That is what causes b2.getPreviousBuild() in the next line 
to go
load b1a, and sets up b1a.nextBuildR to b2, and trigger the assertion error.

Added a code to remove all RunListeners. With this change, the test now fails 
without
the fix in f1430a2.

(cherry picked from commit 7b1b50c85a6a4bdb75f1ae5b40885afa45f96b82)


  Commit: 4bfa16143705e219705ff667c6917a2d6a6a8939
      
https://github.com/jenkinsci/jenkins/commit/4bfa16143705e219705ff667c6917a2d6a6a8939
  Author: Kohsuke Kawaguchi <[email protected]>
  Date:   2014-08-01 (Fri, 01 Aug 2014)

  Changed paths:
    M core/src/main/java/jenkins/model/lazy/BuildReference.java
    M core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java

  Log Message:
  -----------
  [FIXED JENKINS-22395] redoing the fix in f1430a2

Based on the last few commits, I proved that the original fix in f1430a2
doesn't really address the problem.

That is, once b2 is deleted, and after sufficient garbage collection,
we can make b2.previousBuild.get() be null, and then
b2.getPreviousBuild().getNextBuild() ends up incorrectly returning b2.

In this commit, I roll back that part of f1430a2, and then fix the
problem differently.

I started thinking that the main problem we are trying to fix here
is that the deleted build object should be unreferenceable. That is,
it should behave almost as if the object has already been GCed.
The easiest way to do this is to clear a BuildReference object,
since we always use the same BuildReference object for all inbound
references.

This change allows us to clear BuildReference. Code like
b2.getPreviousBuild() will continue to try to update
b1.nextBuildR to b2, but it will only end up wiping out the field,
only to have b1.getNextBuild() recompute the correct value.

This fix makes both test cases pass in LazyBuildMixInTest.

(cherry picked from commit b6226ad2d1a332cb661ceb5c5f5b673771118e14)


  Commit: ca156f730b9aedac9072f7243e3f1528330ca68b
      
https://github.com/jenkinsci/jenkins/commit/ca156f730b9aedac9072f7243e3f1528330ca68b
  Author: Kohsuke Kawaguchi <[email protected]>
  Date:   2014-08-01 (Fri, 01 Aug 2014)

  Changed paths:
    M core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java

  Log Message:
  -----------
  [JENKINS-22395] more diagnostics

Just in case the previous fix didn't address the root cause of ZD-11985 (and 
given the time it takes for changes like this to land in LTS), I'm adding 
additional diagnostics that let us track how previous/next builds are getting 
discovered

(cherry picked from commit aa8e0b4f0e312ba4ef9c647ccf2c093793a9bece)


  Commit: 579753403c63886cf0feff177602d79254829780
      
https://github.com/jenkinsci/jenkins/commit/579753403c63886cf0feff177602d79254829780
  Author: Jesse Glick <[email protected]>
  Date:   2014-08-01 (Fri, 01 Aug 2014)

  Changed paths:
    M test/src/test/java/jenkins/model/lazy/LazyBuildMixInTest.java

  Log Message:
  -----------
  [JENKINS-22395] Taking advantage of BuildReference.clear (just introduced in 
b6226ad) to simplify test by not requiring a custom build reference holder just 
to simulate GC.
Confirmed that dropLinksAfterGC and dropLinksAfterGC2 both fail in the expected 
way (b1a.nextBuild == b2) after commenting out the call to 
createReference().clear() in dropLinks.
(Also that they fail as expected in assertNotSame if the reference is not 
cleared at all.)

(cherry picked from commit b7ec8578f938864af9c8dbfb73a54d67f972d694)


  Commit: 632d7fe0d01d492e0803495691f4397980cf44e0
      
https://github.com/jenkinsci/jenkins/commit/632d7fe0d01d492e0803495691f4397980cf44e0
  Author: Kohsuke Kawaguchi <[email protected]>
  Date:   2014-08-01 (Fri, 01 Aug 2014)

  Changed paths:
    M core/src/main/java/hudson/util/RunList.java
    M core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
    A core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java
    M core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java

  Log Message:
  -----------
  [FIXED JENKINS-18065] made ALLRM.entrySet() smarter

(cherry picked from commit 54c084613f83f9b7a02957eb98dbd8dc661d1c9c)

Conflicts:
        changelog.html


  Commit: f1b0779c6d356db8fb0de6c89bd5bc03483d6b34
      
https://github.com/jenkinsci/jenkins/commit/f1b0779c6d356db8fb0de6c89bd5bc03483d6b34
  Author: Jesse Glick <[email protected]>
  Date:   2014-08-01 (Fri, 01 Aug 2014)

  Changed paths:
    M core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java

  Log Message:
  -----------
  [JENKINS-18065] 54c0846 amendment: presumably 
RunMap.entrySet().iterator().next().setValue(…) should be illegal.
(cherry picked from commit da57ad5c30b1c9ca8365f869638abd29c8fb5896)


  Commit: 8c70a2b76875d2ca6a114c45fd54c0b1e9ecb7d2
      
https://github.com/jenkinsci/jenkins/commit/8c70a2b76875d2ca6a114c45fd54c0b1e9ecb7d2
  Author: Jesse Glick <[email protected]>
  Date:   2014-08-01 (Fri, 01 Aug 2014)

  Changed paths:
    M core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
    M core/src/test/java/jenkins/model/lazy/FakeMap.java

  Log Message:
  -----------
  [JENKINS-18065] Uncommenting original test.
The actual implementation does not behave as nicely as one would hope, for a 
few reasons:
1. isEmpty actually tries to load the newestBuild, rather than simply checking 
whether there is some idOnDisk.
   Arguably this is necessary, since there could be an unloadable build record, 
in which case it would be technically correct for the map to be considered 
empty.
2. Loading AbstractLazyLoadRunMap.newestBuild calls search(MAX_VALUE, DESC), 
which behaves oddly, by doing a binary search, and thus perhaps loading 
lg(|map|) entries.
   You would think that it would suffice to check for the last member of 
idOnDisk in index.byId.
3. The iterator eagerly loads the next value before hasNext has even been 
called.
   Looks bad in a test, though it probably has little practical impact since 
most callers would be calling hasNext soon afterward anyway.
   Might cause one extra build record to be loaded unnecessarily from a limited 
RunList.

(cherry picked from commit 2f58ceb1a66c8ed621fa2562c5ed3445b29ea200)


  Commit: e6a46d880dc7eaf06a6df368b0a42156447c0a6d
      
https://github.com/jenkinsci/jenkins/commit/e6a46d880dc7eaf06a6df368b0a42156447c0a6d
  Author: Jesse Glick <[email protected]>
  Date:   2014-08-01 (Fri, 01 Aug 2014)

  Changed paths:
    M core/src/main/java/hudson/model/DirectoryBrowserSupport.java

  Log Message:
  -----------
  [FIXED JENKINS-20663] For now, go back to using ZipOutputStream from Ant that 
supports setting the filename encoding (present in java.util.zip only in Java 
7+).
(cherry picked from commit 84c76253862a2f36f813a7aa45b77d99c1616be4)

Conflicts:
        changelog.html


  Commit: dbab8f98f78acb4ec6eab1841a50a7a89df32818
      
https://github.com/jenkinsci/jenkins/commit/dbab8f98f78acb4ec6eab1841a50a7a89df32818
  Author: Jesse Glick <[email protected]>
  Date:   2014-08-01 (Fri, 01 Aug 2014)

  Changed paths:
    M pom.xml

  Log Message:
  -----------
  Do not mix old and new test-annotations libraries.
Corrects 9ce6b0fec3c0a0f1b9ea786865b2130a900a5886.
(cherry picked from commit 032a47599d63a38d499b8f015dc57b2d41419130)


Compare: 
https://github.com/jenkinsci/jenkins/compare/559cb4ccd73b...dbab8f98f78a

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Commits" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to