Branch: refs/heads/master
  Home:   https://github.com/jenkinsci/jenkins
  Commit: fb47f1d11346fe45245a8fdb1b2cf9e188d3c0c8
      
https://github.com/jenkinsci/jenkins/commit/fb47f1d11346fe45245a8fdb1b2cf9e188d3c0c8
  Author: Kohsuke Kawaguchi <k...@kohsuke.org>
  Date:   2014-07-10 (Thu, 10 Jul 2014)

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

  Log Message:
  -----------
  update to #index must be synchronized


  Commit: 53e95ee517259e1e10d7209bcbaa4314397c1f40
      
https://github.com/jenkinsci/jenkins/commit/53e95ee517259e1e10d7209bcbaa4314397c1f40
  Author: Kohsuke Kawaguchi <k...@kohsuke.org>
  Date:   2014-07-10 (Thu, 10 Jul 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.


  Commit: 7b1b50c85a6a4bdb75f1ae5b40885afa45f96b82
      
https://github.com/jenkinsci/jenkins/commit/7b1b50c85a6a4bdb75f1ae5b40885afa45f96b82
  Author: Kohsuke Kawaguchi <k...@kohsuke.org>
  Date:   2014-07-10 (Thu, 10 Jul 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.


  Commit: b6226ad2d1a332cb661ceb5c5f5b673771118e14
      
https://github.com/jenkinsci/jenkins/commit/b6226ad2d1a332cb661ceb5c5f5b673771118e14
  Author: Kohsuke Kawaguchi <k...@kohsuke.org>
  Date:   2014-07-10 (Thu, 10 Jul 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.


  Commit: fe3575c176c32aea4a5f33ddb57069a882bd7c71
      
https://github.com/jenkinsci/jenkins/commit/fe3575c176c32aea4a5f33ddb57069a882bd7c71
  Author: Kohsuke Kawaguchi <k...@kohsuke.org>
  Date:   2014-07-10 (Thu, 10 Jul 2014)

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

  Log Message:
  -----------
  Doc improvements


  Commit: 5e7efa5bfc0b3a744bb8b36628e365931fdee9e5
      
https://github.com/jenkinsci/jenkins/commit/5e7efa5bfc0b3a744bb8b36628e365931fdee9e5
  Author: Kohsuke Kawaguchi <k...@kohsuke.org>
  Date:   2014-07-10 (Thu, 10 Jul 2014)

  Changed paths:
    M changelog.html
    M core/src/main/java/hudson/model/Job.java
    M war/src/main/webapp/css/style.css

  Log Message:
  -----------
  Merge branch 'master' of github.com:jenkinsci/jenkins


Compare: 
https://github.com/jenkinsci/jenkins/compare/2214e9cedb3f...5e7efa5bfc0b

-- 
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 jenkinsci-commits+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to