Skip to content
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

HouseArrestService BUG #1115

Closed
huan1936 opened this issue Jul 9, 2024 · 23 comments · Fixed by #1116 or #1127
Closed

HouseArrestService BUG #1115

huan1936 opened this issue Jul 9, 2024 · 23 comments · Fixed by #1116 or #1127

Comments

@huan1936
Copy link

huan1936 commented Jul 9, 2024

Test environment

  • Host OS version. Windows 10
  • Target device model and iOS version. 13 mini 15.0

Describe the bug
HouseArrestService, when initializing, calls the send_command function by default. When the application is unreadable, it triggers
pymobiledevice3.exceptions.PyMobileDevice3Exception: InstallationLookupFailed.
However, the connection is not closed, which will cause the HouseArrestService to be prohibited from creating further connections after approximately 300~600 failures due to too many connections, necessitating a device restart.

To Reproduce
Steps to reproduce the behavior:

  1. Starting the HouseArrestService for applications that do not have UIFileSharingEnabled enabled.
  2. 300~600 consecutive times.
  3. Execute again
  4. See error

Expected behavior
Even if an error occurs, the service should be closed in time

Logs

Traceback (most recent call last):
  File "ttt2.py", line 51, in <module>
    test()
  File "ttt2.py", line 37, in test
    service = HouseArrestService(lockdown=dev.lockdown,
  File "D:\pyCode\other\venv\lib\site-packages\pymobiledevice3\services\house_arrest.py", line 26, in __init__
    self.send_command(bundle_id, cmd)
  File "D:\pyCode\other\venv\lib\site-packages\pymobiledevice3\services\house_arrest.py", line 32, in send_command
    raise PyMobileDevice3Exception(error)
pymobiledevice3.exceptions.PyMobileDevice3Exception: InstallationLookupFailed

Connection forbidden Error log

Traceback (most recent call last):
  File "D:\pyCode\other\ttt2.py", line 50, in <module>
    test()
  File "D:\pyCode\other\ttt2.py", line 37, in test
    service = HouseArrestService(lockdown=dev.lockdown,
  File "D:\pyCode\other\venv\lib\site-packages\pymobiledevice3\services\house_arrest.py", line 18, in __init__
    super().__init__(lockdown, self.SERVICE_NAME)
  File "D:\pyCode\other\venv\lib\site-packages\pymobiledevice3\services\afc.py", line 221, in __init__
    super().__init__(lockdown, service_name)
  File "D:\pyCode\other\venv\lib\site-packages\pymobiledevice3\services\lockdown_service.py", line 20, in __init__
    service = start_service(service_name, include_escrow_bag=include_escrow_bag)
  File "D:\pyCode\other\venv\lib\site-packages\pymobiledevice3\lockdown.py", line 67, in _inner_reconnect_on_remote_close
    return f(*args, **kwargs)
  File "D:\pyCode\other\venv\lib\site-packages\pymobiledevice3\lockdown.py", line 499, in start_lockdown_service
    attr = self.get_service_connection_attributes(name, include_escrow_bag=include_escrow_bag)
  File "D:\pyCode\other\venv\lib\site-packages\pymobiledevice3\lockdown.py", line 489, in get_service_connection_attributes
    response = self._request('StartService', options)
  File "D:\pyCode\other\venv\lib\site-packages\pymobiledevice3\lockdown.py", line 577, in _request
    raise exception_errors.get(error, LockdownError)(error, self.identifier)
pymobiledevice3.exceptions.MissingValueError: MissingValue

Additional context

def test():
    service = HouseArrestService(lockdown=dev.lockdown,
                                 bundle_id='com.tencent.mqq',
                                 documents_only=True)

    service.exists('/Documents')


pool = ThreadPoolExecutor(20)
for _ in range(500):
    pool.submit(test)

pool.shutdown()

For community

⬇️ Please click the 👍 reaction instead of leaving a +1 or 👍 comment

@doronz88
Copy link
Owner

doronz88 commented Jul 9, 2024

Doesn't using it in a context-manager solve all? Sounds like you forgot to close the resource

@huan1936
Copy link
Author

huan1936 commented Jul 9, 2024

Look at this
It is an error in the __init__ function, Function __enter__ has not yet been run.
context-manager won't work

@huan1936
Copy link
Author

huan1936 commented Jul 9, 2024

After simplification

class Test:

    def __init__(self):
        print("Test init")
        raise Exception("Test init")

    def __enter__(self):
        print("Test enter")
        return self

    def __exit__(self, exc_type, exc_val, exc_tb):
        print("Test exit")
        return True


if __name__ == '__main__':
    with Test() as t:
        print("Test with")

@huan1936
Copy link
Author

huan1936 commented Jul 9, 2024

Doesn't using it in a context-manager solve all? Sounds like you forgot to close the resource

So what I'm talking about is that when the UIFileSharingEnabled is not enabled

@doronz88
Copy link
Owner

doronz88 commented Jul 9, 2024

I'm sorry but I'm failing to understand your claim. Did you refactor your code to use a context-manager or did you not? If you did not, the service socket will remain open which could explain your bug.

I don't understand what other behavior you would expect

@huan1936
Copy link
Author

huan1936 commented Jul 9, 2024

UIFileSharingEnabled is not enabled
So when executing send_command, an error will occur, but the service is already connected at this time, and the service is not closed after the error

@doronz88
Copy link
Owner

doronz88 commented Jul 9, 2024

Oh, so you mean the socket remains open till python's GC closes it because the object cannot be created in the first place?

@huan1936
Copy link
Author

huan1936 commented Jul 9, 2024

···
def init(self, lockdown: LockdownServiceProvider, bundle_id: str, documents_only: bool = False):
if isinstance(lockdown, LockdownClient):
super().init(lockdown, self.SERVICE_NAME)
else:
super().init(lockdown, self.RSD_SERVICE_NAME)
if documents_only:
cmd = VEND_DOCUMENTS
else:
cmd = VEND_CONTAINER
self.documents_only = documents_only
self.send_command(bundle_id, cmd) # Error location Here self.service is already connected
···

@doronz88
Copy link
Owner

doronz88 commented Jul 9, 2024

You could simply answer my question.
Anyhow, feel free to submit a PR to fix that

@huan1936
Copy link
Author

huan1936 commented Jul 9, 2024

Python will recycle but Apple has a bug that will prohibit connections if too many services are connected

@doronz88
Copy link
Owner

doronz88 commented Jul 9, 2024

Sounds like a simple try... finally... fix that

@huan1936
Copy link
Author

huan1936 commented Jul 9, 2024

yes

@huan1936
Copy link
Author

huan1936 commented Jul 9, 2024

limited number of connections.
Therefore, you must implement try... finally... in __init__, otherwise you cannot use context-manager

@huan1936
Copy link
Author

huan1936 commented Jul 9, 2024

Or remove self.send_command(bundle_id, cmd) in __init__

@doronz88
Copy link
Owner

doronz88 commented Jul 9, 2024

I prefer the try finally since otherwise the service practically will be of no use and this service object has no meaning

@huan1936
Copy link
Author

huan1936 commented Jul 9, 2024

Yes, so please fix it.
I have currently added a try-finally block after inheriting.
Do not forget to add a check to see if the self.service attribute exists.
In special cases, self.service may not be present.

@doronz88 doronz88 linked a pull request Jul 9, 2024 that will close this issue
@huan1936 huan1936 closed this as completed Jul 9, 2024
doronz88 added a commit that referenced this issue Jul 9, 2024
house_arrest: trigger `close()` when failing in ctor (#1115)
@huan1936 huan1936 reopened this Jul 18, 2024
@huan1936
Copy link
Author

You can't write like this
This will cause it to close immediately after initialization.

@huan1936
Copy link
Author

He should be like this

image

@huan1936
Copy link
Author

And why not use del to ensure that it is closed when recycled?

@doronz88
Copy link
Owner

Overriding__del__ is not a good practice. I'm failing to see the problem you are pointing at. If you have a reproduceable issue, please explain.

@huan1936
Copy link
Author

with HouseArrestService(lockdown=dev.lockdown,
                             bundle_id='com.tencent.mqq',
                             documents_only=True) as service:
    print(service.exists('/Documents'))  # Expected: True  # Actual: [WinError 10038] 

Because the service has been shut down.
So service.exists('/Documents') cannot be executed

@huan1936
Copy link
Author

    def __init__(self, lockdown: LockdownServiceProvider, bundle_id: str, documents_only: bool = False):
        if isinstance(lockdown, LockdownClient):
            super().__init__(lockdown, self.SERVICE_NAME)
        else:
            super().__init__(lockdown, self.RSD_SERVICE_NAME)
        if documents_only:
            cmd = VEND_DOCUMENTS
        else:
            cmd = VEND_CONTAINER
        self.documents_only = documents_only
        try:
            self.send_command(bundle_id, cmd)
        finally:
            self.close()  # close() is called even if no exception occurs.

doronz88 added a commit that referenced this issue Jul 18, 2024
house_arrest: trigger `close()` only upon error (#1115)
@huan1936
Copy link
Author

Overriding__del__ is not a good practice. I'm failing to see the problem you are pointing at. If you have a reproduceable issue, please explain.覆盖 __del__ 不是一个好的做法。我没有看到你指出的问题。如果您有可重现的问题,请解释。

So what about this?

    def __init__(...):
         self._finalizer = weakref.finalize(self, self.service.close)


    def close(self):
        self._finalizer()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants