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

Panoramix fails to work not in main thread (and does not work in Windows) #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions panoramix/decompiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import sys
from contextlib import redirect_stdout

import timeout_decorator
from wrapt_timeout_decorator import timeout

import panoramix.folder as folder
from panoramix.contract import Contract
Expand Down Expand Up @@ -155,7 +155,7 @@ def _decompile_with_loader(loader, only_func_name=None) -> Decompilation:
if target > 1 and loader.lines[target][1] == "jumpdest":
target += 1

@timeout_decorator.timeout(60 * 3, timeout_exception=TimeoutInterrupt)
@timeout(60 * 3, timeout_exception=TimeoutInterrupt, use_signals=False)
Copy link

@ytrezq ytrezq Aug 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nizak : performance wise, should this be made conditionnal based on the plateform not being Unix ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need it in Linux Ubuntu and in Macos because I use panoramix in a separate thread

Copy link

@ytrezq ytrezq Aug 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NikZak : then the cost of using detecting this if it’s not in the main thread is very low compared to checking the Timemout twice every time.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Although I reckon initializing a process is a miniscule part of the whole decompiling procedure.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that writing something like "if platform is windows or if runing not in the main thread" is brittle and kind of ugly.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. It’s something wanted for large contracts.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

def dec():
trace = VM(loader).run(target, stack=stack, timeout=60)
explain("Initial decompiled trace", trace[1:])
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"coloredlogs",
"requests",
"web3",
"timeout_decorator",
"wrapt_timeout_decorator",
"appdirs",
]

Expand Down