Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LPS-95225 Tracks Liferay portlet services rather than javax.portlet instances #73705

Closed
wants to merge 1 commit into from

Conversation

@drewbrokke
Copy link

commented May 24, 2019

This is an update for LPS-95225.

Hey Brian, this solves a race condition where multiple javax.portlet.Portlet trackers would call back in the wrong order. Only PortletTracker should track javax.portlet.Portlet instances, and every other tracker should track com.liferay.portal.kernel.model.Portlet. That way there is a guarantee that all the portlet setup logic has run first, and in this case it guarantees that the portlet has been persisted to the Portlet table.

@hhuijser is it reasonable to add a SF rule for this?

/cc @pei-jung @stsquared99 @alee8888 @ChrisKian @dejuknow @ctampoya

LPS-95225 application-list-api - track com.liferay.portal.kernel.mode…
…l.Portlet so that the portlet is guaranteed to be persisted to the Portlet table before PanelAppRegistry queries for it
@liferay-continuous-integration

This comment has been minimized.

Copy link
Collaborator

commented May 24, 2019

CI is automatically triggering "ci:test:sf" and "ci:test:relevant" for this pull to run Source Formatter and relevant tests.

Comment "ci:test" to run the full PR Tester for this pull.

@liferay-continuous-integration

This comment has been minimized.

Copy link
Collaborator

commented May 24, 2019

✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutes 8 seconds 109 ms

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: ec2b3a679e784fd2bc0434fb5d7a0a08165f071e

Sender Branch:

Branch Name: LPS-95225
Branch GIT ID: 41355d0bd57e8c7a715b2261fa6debf38148c734

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.
@brianchandotcom

This comment has been minimized.

Copy link
Owner

commented May 24, 2019

@Preston-Crary fyi on PortletTracker.

@liferay-continuous-integration

This comment has been minimized.

Copy link
Collaborator

commented May 24, 2019

ci:test:relevant - 19 out of 21 jobs passed in 1 hour 35 minutes 31 seconds 44 ms

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: ec2b3a679e784fd2bc0434fb5d7a0a08165f071e

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: ec9e53a4905a536a6e2d94427a2dab12b43e807e

19 out of 21 jobs PASSED
19 Successful Jobs:
For more details click here.

Failures unique to this pull:

  1. test-portal-acceptance-pullrequest-batch(master)/functional-upgrade-tomcat90-mysql57-jdk8/1
    Job Results:

    0 Tests Passed.
    1 Test Failed.

    1. AXIS_VARIABLE=0,label_exp=!master #300241
           [echo]    java.lang.Thread.State: RUNNABLE
           [echo] 
           [echo]    Locked ownable synchronizers:
           [echo] 	- None
           [echo] 
           [echo] "Finalizer" #3 daemon prio=8 os_prio=0 tid=0x00007f6c900a7800 nid=0x3d16 in Object.wait() [0x00007f6c94e12000]
           [echo]    java.lang.Thread.State: WAITING (on object monitor)
           [echo] 	at java.lang.Object.wait(Native Method)
           [echo] 	- waiting on <0x00000000820a68b0> (a java.lang.ref.ReferenceQueue$Lock)
           [echo] 	at java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:143)
           [echo] 	- locked <0x00000000820a68b0> (a java.lang.ref.ReferenceQueue$Lock)
           [echo] 	at java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:164)
           [echo] 	at java.lang.ref.Finalizer$FinalizerThread.run(Finalizer.java:209)
           [echo] 
           [echo]    Locked ownable synchronizers:
           [echo] 	- None
           [echo] 
           [echo] "Reference Handler" #2 daemon prio=10 os_prio=0 tid=0x00007f6c900a2800 nid=0x3d13 in Object.wait() [0x00007f6c94f13000]
           [echo]    java.lang.Thread.State: WAITING (on object monitor)
           [echo] 	at java.lang.Object.wait(Native Method)
           [echo] 	- waiting on <0x00000000820a68f0> (a java.lang.ref.Reference$Lock)
           [echo] 	at java.lang.Object.wait(Object.java:502)
           [echo] 	at java.lang.ref.Reference.tryHandlePending(Reference.java:191)
           [echo] 	- locked <0x00000000820a68f0> (a java.lang.ref.Reference$Lock)
           [echo] 	at java.lang.ref.Reference$ReferenceHandler.run(Reference.java:153)
           [echo] 
           [echo]    Locked ownable synchronizers:
           [echo] 	- None
           [echo] 
           [echo] "VM Thread" os_prio=0 tid=0x00007f6c9009b000 nid=0x3d12 runnable 
           [echo] 
           [echo] "Gang worker#0 (Parallel GC Threads)" os_prio=0 tid=0x00007f6c9001c800 nid=0x3d10 runnable 
           [echo] 
           [echo] "Gang worker#1 (Parallel GC Threads)" os_prio=0 tid=0x00007f6c9001e000 nid=0x3d11 runnable 
           [echo] 
           [echo] "VM Periodic Task Thread" os_prio=0 tid=0x00007f6c902f6000 nid=0x3d22 waiting on condition 
           [echo] 
           [echo] JNI global references: 2809
            [get] Getting: http://test-1-19/job/test-portal-acceptance-pullrequest-batch(master)/AXIS_VARIABLE=0,label_exp=!master/300241//consoleText
            [get] To: /opt/dev/projects/github/liferay-portal/20190524225106254.txt
         [delete] Deleting: /opt/dev/projects/github/liferay-portal/20190524225106254.txt
         [delete] Deleting: /opt/dev/projects/github/liferay-portal/null562998547.properties

For upstream results, click here.

@hhuijser

This comment has been minimized.

Copy link

commented May 27, 2019

@drewbrokke what's the rule?
If a class implements ServiceTrackerCustomizer and Portlet is one of the generic types?

@drewbrokke

This comment has been minimized.

Copy link
Author

commented May 28, 2019

@hhuijser that's correct, but specifically:

  • com.liferay.portal.kernel.model.Portlet is allowed
  • javax.portlet.Portlet is NOT allowed, except in PortletTracker
@hhuijser

This comment has been minimized.

Copy link

commented May 30, 2019

@drewbrokke should this be backported?

@drewbrokke

This comment has been minimized.

Copy link
Author

commented Jun 5, 2019

@hhuijser do you mean should the SF rule be backported, or the changes in this pull?

@hhuijser

This comment has been minimized.

Copy link

commented Jun 5, 2019

@drewbrokke
Should we only check for this on master or also check and fix any cases in other branches?

@drewbrokke

This comment has been minimized.

Copy link
Author

commented Jun 6, 2019

Ah, I see. I think it makes sense to check other branches. Since we're backporting the fix on this one, we can backport the check also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.