Skip to content

Conversation

@maximpavliv
Copy link
Contributor

In my previous attempt to add a timeout to deeplabcut.gui.window._check_for_updates(), the tests were failing on Windows and MacOS due to a difference in the multiprocessing package workflow across these different OSes. This attempt has therefore been reverted.

This Pull Requests proposes an addition of timeout to _check_for_updates() with tests that should be compatible with all 3 OS families (tested on Ubuntu 24.04 and Windows, @n-poulsen could you confirm this also works on MacOS please?).

@maximpavliv
Copy link
Contributor Author

For info:
The difference in multiprocessing between Linux and Windows or MacOS is how it starts new processes (by default):

  • On Linux, the default method is "fork"
  • On Windows and MacOS, the default method is "spawn"

This can be verified by doing

import multiprocessing
multiprocessing.get_start_method()

With "fork", the child process inherits the memory space, file descriptors, and environment from the parent process. With "spawn", a fresh Python interpreter is started, and only the necessary objects (like the target function) are passed to the child process.
Therefore, to successfully run the tests on all OSes, the methods passed to multiprocessing cannot be defined in a local scope, because they cannot be pickled if it is the case, which then is not compatible with the "spawn" method.
Another consequence is that, in practice, deeplabcut needs to be reimported in each child process if using the "spawn" method (both when running test_call_with_timeout() and when running _check_for_updates()), which might take some time. This is not the case if new processes are started using "fork" method (Linux).
@n-poulsen do you see any potential issue with this, that I didn't think of?

@maximpavliv maximpavliv force-pushed the maxim/fix_version_check_timeout_2 branch from 8455f59 to 84ff599 Compare January 10, 2025 07:16
@maximpavliv
Copy link
Contributor Author

@n-poulsen I increased the timeout from 1s to 5s in version checks, what do you think about this?

@MMathisLab MMathisLab merged commit 943b728 into pytorch_dlc Feb 15, 2025
1 check was pending
@MMathisLab MMathisLab deleted the maxim/fix_version_check_timeout_2 branch February 15, 2025 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants