Closed Bug 1340099 Opened 7 years ago Closed 6 years ago

GeckoDriver gets instantiated multiple times during startup

Categories

(Remote Protocol :: Marionette, defect, P3)

Version 3
defect

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1169290

People

(Reporter: whimboo, Unassigned)

References

Details

You can see this very easily when running marionette with the --jsdebugger argument option. There will be three alert boxes you will have to accept before running the tests.

When I added some print/dump statements to the appropriate Python and JS code lines the following output is given:

**** INIT gecko driver
Profile path is /var/folders/4k/sf4gz5fn3kl9hr3nd7pzbvhc0000gn/T/tmpcgc2eO.mozrunner
Starting fixture servers
----- start session ------
**** INIT gecko driver
**** INIT gecko driver

It means we even create an instance of GeckoDriver before we actually start a session. This might be fine, but why do we create two more instances with calling start_session() on the Python side?
I don't see why we have
It looks like that this gets initiated in MarionetteServer.prototype.onSocketAccepted(), which indicates that we have three different connection attempts to Marionette.

And I see when using --gecko-log -vv:

1487241582852	Marionette	DEBUG	Accepted connection conn0 from 127.0.0.1:59842
Profile path is /var/folders/4k/sf4gz5fn3kl9hr3nd7pzbvhc0000gn/T/tmpjhAX_2.mozrunner
Starting fixture servers
----- start session ------
1487241582928	Marionette	DEBUG	Closed connection conn0
**** INIT gecko driver
1487241582930	Marionette	DEBUG	Accepted connection conn1 from 127.0.0.1:59843
1487241583093	Marionette	DEBUG	Closed connection conn1
**** INIT gecko driver
1487241583100	Marionette	DEBUG	Accepted connection conn2 from 127.0.0.1:59844

Andreas, I wonder if this could be a problem during startup of Firefox. At least for debugging it's annoying.

Could this come from our checks if Marionette is running on a specific port in the Python code?
Flags: needinfo?(ato)
Oh, and we also run GeckoDriver:newSession() multiple times, which means that we modify global variables very early in the startup process. We also add event listeners and observer notifications which might not get removed.

I wonder if this somehow might be related to bug 1315611.
The first connection, conn0, happens as you say when we check if port 2828 is open.  The only way to test if a port is open is by connecting to it.  Connecting to the Marionette server causes a new connection to be created, which again constructs a new instance of the GeckoDriver class.

In my opinion, the fact that GeckoDriver gets constructed is not in itself a problem.  The problem is that its constructor modifies global state and one would not expect a class constructor to have repercussions like that.

Looking a bit ahead, I imagine a future where the Marionette server provides more compartmentalisation and control over what is provided, both to reduce the overhead of what is available by default (which this bug is an example of) and to prevent GeckoDriver from turning into a God object.  I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1330309 about this some time ago, and it may be a good place to fix this.

In this future, the following would take place:

  1. When a new connection is made, a new Dispatcher is constructed and assigned for each new socket.

  2. It exposes a basic ‘Marionette’ service that consists of a few basic commands: ‘Connect’, ‘Disconnect’, ‘Enable Service’, and ‘Disable Service’.

  2. GeckoDriver is not instantiated by default, and clients must call an ‘Enable Service’ command to enable different Marionette services.  ‘WebDriver’ might be one such service.  Other services are ‘L10n’, ‘Addons’, et al.

An instantiation of WebDriver might look like this:

  - Connect socket to port 2828
  - Send ‘Connect’ command
  - Send ‘Enable Service’ command with argument ‘WebDriver’
  - …
  - Send ‘Disable Service’ command with argument ‘WebDriver’
  - Send ‘Disconnect’ command
  - Close socket

Anyway, dealing with the realities of today: I don’t think we are seeing any sort of disastrous fallout by instantiating GeckoDriver multiple times, although we should probably be more careful about the state we generate in its ctor.  The only way in which I can see us not constructing GeckoDriver on connecting is by following an approach as I describe above.  To rephrase: The only way we can ensure GeckoDriver isn’t constructed multiple times is to make the construction of it explicit.
Flags: needinfo?(ato)
Blocks: 1296628
Priority: -- → P3
I believe this was resolved by bug 1169290.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.