Skip to content

Conversation

@dinana
Copy link

@dinana dinana commented Jan 20, 2026

Summary

  • Port the wait_x_socket fix from fix: x11vnc race condition with Xvfb. #350 to stable/ scripts
  • Use display :99 to avoid potential conflicts with host X servers
  • Add VNC_PORT environment variable to configure VNC server port
  • Add file_env 'TWS_USERID' support for Docker secrets

Changes

stable/scripts/common.sh

  • Add wait_x_socket() function (same as image-files)

stable/scripts/run.sh

  • Use display :99 instead of :1
  • Clean up stale X lock and socket files on startup
  • Call wait_x_socket after starting Xvfb
  • Add VNC_PORT env var support (default 5900)
  • Use $DISPLAY variable in x11vnc for consistency

image-files/scripts/run.sh

  • Add VNC_PORT env var support

image-files/scripts/common.sh

  • Add file_env 'TWS_USERID' for Docker secrets support

Related

stable/scripts:
- Use display :99 to avoid conflicts with host X servers
- Add wait_x_socket function to ensure Xvfb ready before VNC
- Kill existing Xvfb processes on startup for clean restarts
- Use $DISPLAY variable in x11vnc for consistency
- Add VNC_PORT env var support (default 5900)

image-files/scripts:
- Add file_env support for TWS_USERID
- Add VNC_PORT env var support
@gnzsnz
Copy link
Owner

gnzsnz commented Jan 20, 2026

Hi,

thanks for taking the time and submitting this PR. I have review it and I do have some questions that I would like to understand before moving forward.

Port the wait_x_socket fix from #350 to stable/ scripts

This one for me is clear. I won't apply it, so please roll it back. Every release is published at the moment they came out, and I don't update them. Unless there is a serious problem. I don't consider this issue too serious. If you are facing this issue, you can build stable locally. Next stable release will include the fix. To put it in another way, no PR should touch stable or latest directories.

Use display :99 to avoid potential conflicts with host X servers

I would need a good explanation to implement this one. AFAIK there is zero chance that the display :1 will conflict with the host. A container is an isolated environment, how could this happen? Can you provide a good rationale and reproduction steps? If not please roll this back.

Add VNC_PORT environment variable to configure VNC server port

Again, I would need a good rationale to apply this. What is the problem that you are trying to solve here? Why do you need to change the VNC port? The default port works, is well-known(TM) and I don't see a reason to change it. It won't conflict with anything within the container. If you need to change the port to the outside, docker provides tools to do that. Please provide a good rationale.

Add file_env 'TWS_USERID' support for Docker secrets

This one could be applied but unfortunately userid is no "secret" it's all over the place on logs. So what are we gaining here?

IBC: detected frame entitled: U1234567 Interactive Brokers; event=Lost focus

Let me know your comments.

@dinana
Copy link
Author

dinana commented Jan 20, 2026

Port the wait_x_socket fix from #350 to stable/ scripts

Agreed, will wait for you to apply to stable

Use display :99 to avoid potential conflicts with host X servers

Add VNC_PORT environment variable to configure VNC server port

The use case is when using in some air-tight environments, container can use HOST networking, simplifying integration by avoiding ssh altogether (and latency) while maintaining the same level of security (port access controlled by host).

Add file_env 'TWS_USERID' support for Docker secrets

Regarding this (as well as the above 2) it's adding further customizability to allow for different use cases without affecting any of the existing functionality, it's all optional.

@gnzsnz
Copy link
Owner

gnzsnz commented Jan 26, 2026

I have reviewed the changes and your comments. And I won't implement it, there is no reason to add these new variables, the same result can be achieved using standard docker networking.

The only thing that should be applied is start_vnc function that should use DISPLAY, but it should be applied on run.sh under image-files/scripts/run.sh.

If you are OK with this, please roll back the changes and submit only a change on image-files/scripts/run.sh line 64

x11vnc -ncache_cr -display :1 -forever -shared -bg -noipv6 -passwd "$VNC_SERVER_PASSWORD" &

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants