search

frida / frida-python

Frida Python bindings
Other
766 stars 145 forks source link

API cleanup #228

Open yotamN opened 1 year ago

yotamN commented 1 year ago

The current API is a bit messy, especially compared to comparative wrappers like frida-node. I think we have three main areas where we should focus our cleanup one:

  1. Core and init merge - if there is a reason for the split I would love to know but as it seems, the split only make the API confusing and verbose.
  2. Uniform interface - we have the C wrapper of Frida in src/_frida.c and the Python wrapper of the C wrapper at frida/core.py. I think we should try to merge them when we can, sometimes we have Python classes that are just dumb wrappers. When we can't merge them, we should only expose the Python wrapper no matter what, otherwise it become confusing really fast.
  3. Async everything - We only support Python 3.7+ and soon will drop support for 3.7 (EOF in June), so we can use the "new" asyncio package. We would keep the sync methods with a _sync prefix, at least I think we should.

The big question of course is how to handle this cleanup, it will break all usages of the library but it's probably for the best.

oleavr commented 1 year ago

The current API is a bit messy, especially compared to comparative wrappers like frida-node. I think we have three main areas where we should focus our cleanup one:

  1. Core and init merge - if there is a reason for the split I would love to know but as it seems, the split only make the API confusing and verbose.

Agreed.

  1. Uniform interface - we have the C wrapper of Frida in src/_frida.c and the Python wrapper of the C wrapper at frida/core.py. I think we should try to merge them when we can, sometimes we have Python classes that are just dumb wrappers. When we can't merge them, we should only expose the Python wrapper no matter what, otherwise it become confusing really fast.

Sounds good.

  1. Async everything - We only support Python 3.7+ and soon will drop support for 3.7 (EOF in June), so we can use the "new" asyncio package. We would keep the sync methods with a _sync prefix, at least I think we should.

Love it!

The big question of course is how to handle this cleanup, it will break all usages of the library but it's probably for the best.

The next opportunity to do that will be Frida 17. The breakage is unfortunate, but OTOH it will completely eliminate a whole class of threading issues that I'm sure affect a lot of existing applications. (The current constraints are not really documented and thus poorly understood — but application developers shouldn't need to worry about such things to begin with.)

Longer term we should probably consider having bindings be versioned separately. We will need a lot of CI/automation in place for that to become practical though.

yotamN commented 1 year ago

Could we maybe use Cython instead of having part of the library written in C and part written in Python? I haven't got to use it really but it seems to be a mature solution with many real world examples like Numpy and Pandas.