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

Connectivity checker causes further "port already in use" issue #2090

Closed
foosel opened this Issue Aug 23, 2017 · 2 comments

Comments

1 participant
@foosel
Owner

foosel commented Aug 23, 2017

Problem

As mentioned in #2038 (comment), there's still an issue with the connectivity checker causing "port already in use" issues on server startup.

The problem can only be observed on Windows (for Linux/Unix the intermediary server is now started in a way that it no longer inherits its socket to subprocesses - sadly that is not an option for Windows). It's caused by the connectivity check triggering an event during runtime of the intermediary server that in turn can trigger several subprocesses in the Software Update Plugin (for checking pip). If the timing is just right that causes the same "port already in use" issue on startup that was reported in #2035 and caused there by the file analysis.

Solution

Current idea to solve: Do not start connectivity check until after the actual tornado server runs. Additionally limit event propagation to plugins that have already been initialized (that was also a problem observed while debugging this further).

In the long run maybe look into reusing the same listening socket for both the intermediary server and tornado (might not be possible however and also feels like a hack).

@foosel foosel added this to the 1.3.5 milestone Aug 23, 2017

@foosel foosel self-assigned this Aug 23, 2017

foosel added a commit that referenced this issue Aug 23, 2017

foosel added a commit that referenced this issue Aug 23, 2017

foosel added a commit that referenced this issue Aug 23, 2017

set_close_exec on intermediary server port for unix & windows
Using the win32 API it's possible to prevent the intermediary server
socket from inheriting itself to subprocesses. So let's use that here.

Another bit of the solution for #2090.
@foosel

This comment has been minimized.

Show comment
Hide comment
@foosel

foosel Aug 23, 2017

Owner

Solved differently than originally anticipated:

  • We no longer fire events before the "Startup" event has been seen. Anything sent before that gets buffered and sent after the startup event. That way nothing generating events during runtime of our intermediary server can have unforeseen side effects.
  • We no longer forward events to uninitialized EventHandlerPlugin implementations.
  • And most importantly, we can now tell the intermediary server socket to NOT inherit itself to any spawned subprocesses under Windows as well (which severely decreases the likelihood of this problem - usually either fcntl or the windows API should be available).
Owner

foosel commented Aug 23, 2017

Solved differently than originally anticipated:

  • We no longer fire events before the "Startup" event has been seen. Anything sent before that gets buffered and sent after the startup event. That way nothing generating events during runtime of our intermediary server can have unforeseen side effects.
  • We no longer forward events to uninitialized EventHandlerPlugin implementations.
  • And most importantly, we can now tell the intermediary server socket to NOT inherit itself to any spawned subprocesses under Windows as well (which severely decreases the likelihood of this problem - usually either fcntl or the windows API should be available).
@foosel

This comment has been minimized.

Show comment
Hide comment
@foosel

foosel Oct 17, 2017

Owner

1.3.5 was released yesterday.

Owner

foosel commented Oct 17, 2017

1.3.5 was released yesterday.

@foosel foosel closed this Oct 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment