fix crash in some rare eventlet edge cases#71
Conversation
|
I am not really a fan of this approach of ignoring attribute errors and I think in the future it will create more problem than it solves. Now looking at the traceback I wonder if this problem is not simply caused by manhole trying to import eventlet too eagerly. Would having a check in sys.modules for eventlet (eg: only try to import it if it was already imported) solve your problem? Shouldn't need to import eventlet/gevent if they didn't patch anything anyway (eg: them not being in sys.modules means we get the original stuff). Can you try this approach? |
|
Now looking at the traceback I wonder if this problem is not simply caused by manhole trying to import eventlet too eagerly. Would having a check in sys.modules for eventlet (eg: only try to import it if it was already imported) solve your problem? Shouldn't need to import eventlet/gevent if they didn't patch anything anyway (eg: them not being in sys.modules means we get the original stuff). Can you try this approach?
Makes sense, will give that a shot.
|
|
i tried that, let me know if it's what you think. i don't have any gevent or eventlet code here to test this path, but the "normal" path works with this. |
|
ping? still looking at dropping this patch from debian, although we still ship the older version of this. |
|
Damn it's been a while. Should have pinged me sooner. I have tried your changes now and there's some problems with it (_get_original is not defined anymore). Maybe I was waiting to hear something about the failed CI and that's why I did not follow up on this PR. Anyway, lets try this change first. |
|
@anarcat does that look like the os packaging patch you have? |
hey so no, this is not the patch we have in Debian anymore. i have modified the patch to match your comments here but failed to refresh it in Debian accordingly: i can try to pull it back in Debian if that's useful for you, otherwise i encourage you to just do as you see fit and i'll adjust when a new release hits github. :) i'm sorry i don't have more time to handle this right now, LLM armies are attacking from all sides. |
Closes: #70