Closed Bug 1435500 Opened 4 years ago Closed 4 years ago

Crash in java.lang.IllegalStateException: Current session is open at org.mozilla.gecko.GeckoView.setSession(GeckoView.java)

Categories

(GeckoView :: General, defect)

58 Branch
Unspecified
Android
defect
Not set
critical

Tracking

(firefox58 wontfix, firefox59 fixed, firefox60 fixed)

RESOLVED FIXED
mozilla60
Tracking Status
firefox58 --- wontfix
firefox59 --- fixed
firefox60 --- fixed

People

(Reporter: philipp, Assigned: droeh)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file, 2 obsolete files)

This bug was filed from the Socorro interface and is
report bp-e2947432-16b7-434b-820f-7fd7b0180131.
=============================================================

Java Stack Trace 	

java.lang.IllegalStateException: Current session is open
	at org.mozilla.gecko.GeckoView.setSession(GeckoView.java:194)
	at org.mozilla.gecko.GeckoApp.onCreate(GeckoApp.java:1037)
	at org.mozilla.gecko.BrowserApp.onCreate(BrowserApp.java:653)
	at android.app.Activity.performCreate(Activity.java:6999)
	at android.app.Activity.performCreate(Activity.java:6990)
	at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1214)
	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2731)
	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2856)
	at android.app.ActivityThread.handleRelaunchActivity(ActivityThread.java:4699)
	at android.app.ActivityThread.-wrap18(Unknown Source:0)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1595)
	at android.os.Handler.dispatchMessage(Handler.java:106)
	at android.os.Looper.loop(Looper.java:164)
	at android.app.ActivityThread.main(ActivityThread.java:6494)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:440)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:807)
	at de.robv.android.xposed.XposedBridge.main(XposedBridge.java:108)

=============================================================

this crash signature starting on fennec 58 seems to be a regression from bug 1413698
Snorp, is this important enough to try to get into 59?
Flags: needinfo?(snorp)
Should be an easy fix.

Dylan I think setSession() should be fine with a session that is already open. Can you write that patch and uplift to 59?
Flags: needinfo?(snorp) → needinfo?(droeh)
I think it suffices here just to remove the existing check. I tested by modifying custom tabs to create a second GeckoSession and set the GeckoView's session to the new one -- things work as expected.
Assignee: nobody → droeh
Flags: needinfo?(droeh)
Attachment #8949415 - Flags: review?(snorp)
Attachment #8949415 - Flags: review?(snorp) → review+
Comment on attachment 8949415 [details] [diff] [review]
Allow setting a new session on a GeckoView

We will leak an open session here meaning we will leak an open nsWindow, which is bad. We should investigate why this is happening rather than just ignore the leak.
Attachment #8949415 - Flags: feedback-
Duplicate of this bug: 1440625
I can't reproduce this, but it seems like it may be similar to bug 1434126, where we were not closing the GeckoSession when PWAs/Custom Tabs were killed. If so, this should fix it.
Attachment #8949415 - Attachment is obsolete: true
Attachment #8954448 - Flags: review?(nchen)
Comment on attachment 8954448 [details] [diff] [review]
Call GeckoSession.closeWindow() in GeckoApp.onDestroy()

I'm not sure we want to always close the session. Why not do this in onCreate before `mLayerView.setSession`?
Attachment #8954448 - Flags: review?(nchen) → feedback+
Fair enough, is there something specific the session might need to do after onDestroy()?
Attachment #8954448 - Attachment is obsolete: true
Attachment #8954842 - Flags: review?(nchen)
Comment on attachment 8954842 [details] [diff] [review]
Close the existing GeckoSession (if any) in GeckoApp.onCreate()

Please add a comment.
Attachment #8954842 - Flags: review?(nchen) → review+
Pushed by droeh@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9f4d34d9451
Close the existing GeckoSession if it exists during GeckoApp.onCreate()  r=jchen
https://hg.mozilla.org/mozilla-central/rev/e9f4d34d9451
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Please request release approval on this ASAP if you think it should be uplifted to 59 still.
Flags: needinfo?(droeh)
Comment on attachment 8954842 [details] [diff] [review]
Close the existing GeckoSession (if any) in GeckoApp.onCreate()

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: Potential start-up crash
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Behavior should only change if we were in a state that would cause a crash.
[String changes made/needed]: None
Flags: needinfo?(droeh)
Attachment #8954842 - Flags: approval-mozilla-release?
Attachment #8954842 - Flags: approval-mozilla-beta?
Comment on attachment 8954842 [details] [diff] [review]
Close the existing GeckoSession (if any) in GeckoApp.onCreate()

Fixes a startup crash by adding code to check for being in a known-bad state. Approved for Fennec 59b15.
Attachment #8954842 - Flags: approval-mozilla-release?
Attachment #8954842 - Flags: approval-mozilla-release+
Attachment #8954842 - Flags: approval-mozilla-beta?
Attachment #8954842 - Flags: approval-mozilla-beta+
See Also: → 1468403
Product: Firefox for Android → GeckoView
Version: Firefox 58 → 58 Branch
Keywords: crash, regression
Target Milestone: Firefox 60 → mozilla60
You need to log in before you can comment on or make changes to this bug.