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

exit() in a pythonscript causes Notepad++ to CRASH #301

Open
victorel-petrovich opened this issue Aug 1, 2023 · 13 comments
Open

exit() in a pythonscript causes Notepad++ to CRASH #301

victorel-petrovich opened this issue Aug 1, 2023 · 13 comments

Comments

@victorel-petrovich
Copy link

victorel-petrovich commented Aug 1, 2023

Using any of these in a pythonscript:

exit(), quit(), sys.exit(), os._exit(0) 

Causes N++ to exit abruptly crash.

Notepad++ v8.5.4 (32-bit)
Build time : Jun 17 2023 - 20:40:14
Path : C:\Users\Vic\AppData\npp.8.5.4.portable\notepad++.exe
Command Line :
Admin mode : OFF
Local Conf mode : ON
Cloud Config : OFF
OS Name : Windows 7 Ultimate (32-bit)
OS Build : 7601.24546
Current ANSI codepage : 1252
Plugins :
mimeTools (2.9)
NppConverter (4.5)
NppExec (0.8.4)
NppExport (0.4)
PythonScript (2)

@victorel-petrovich victorel-petrovich changed the title [bug?] exiting a pythonscript causes Notepad++ to exit [bug?] exit() in a pythonscript causes Notepad++ to exit Aug 1, 2023
@victorel-petrovich
Copy link
Author

victorel-petrovich commented Aug 2, 2023

I found it mentioned in forum , where workarounds for exiting the script are given (like the good def(main): ... main() ).

However, IMHO, this bug can still pose a problem for beginners / unsuspecting people, cannot it? N++ appears like crashing in that moment.

I'm not a python expert, but can't this problem be solved/prevented by, for example, disabling exit() and friends in the provided default startup.py? Like setting exit=0 or

def exit():
   console.write("exit() has been disabled \n")
   return

@Ekopalypse
Copy link
Contributor

I wouldn't call this a bug, it does what it is supposed to do, which is to terminate the running program, which in this case is Npp. It may not be what someone expects if they are not familiar with Python, but it is what it is. Maybe there should be a highlighted note in the document explaining this, but I personally wouldn't start implementing code that prevents you from doing something like this.

@alankilborn
Copy link

def exit():
console.write("exit() has been disabled \n")
return

This is not a bad idea, but for the individual. Meaning that sure, go ahead and implement it yourself in your startup.py. I've done so for mine. BTW, you don't need the return.

@victorel-petrovich
Copy link
Author

victorel-petrovich commented Aug 3, 2023

@Ekopalypse

I wouldn't call this a bug, it does what it is supposed to do, which is to terminate the running program, which in this case is Npp. It may not be what someone expects if they are not familiar with Python, but it is what it is.

Those commands are supposed/meant/defined to terminate Python, not N++... So I don't see why people good at Python would think terminating N++ is expected.
IMO, the natural expectation is that when Python is "killed", control to return to N++ which to continue running. It's not like N++ were running "inside" a running Python interpreter, or something like that.

Maybe there should be a highlighted note in the document explaining this,

Totally, I can do that.

@alankilborn

This is not a bad idea, but for the individual.

Well, for all individuals., to be already in their startup.py, when they install this plugin, no?
What's the point of me or you doing that if I'm already familiar with this "bug" ? (unless I forget about it later, I guess).

BTW, you don't need the return.

you're right!

@alankilborn
Copy link

...Or, just don't use exit(). In a decade of using Python, I've never used this function in code. There are other and better alternatives. Some of this was discussed on the Community forum, as I believe you've discovered.

@Ekopalypse
Copy link
Contributor

Those commands are supposed/meant/defined to terminate Python, not N++... So I don't see why people good at Python would think terminating N++ is expected.
IMO, the natural expectation is that when Python is "killed", control to return to N++ which to continue running. It's not like N++ were running "inside" a running Python interpreter, or something like that.

Not if you understand that it does not stop a runtime per se, but the process that provides that runtime. PS does not start a separate Python process to run PythonScripts but provides the runtime inside the Npp process, so from my point of view it is expected that an exit terminates Npp. To date, Python has no mechanism for stopping AND starting runtimes/interpreters. It is initialized (started) and runs until it is terminated (end of process). There are plans to add a functionality called subinterpreter, which would then have to provide such a function, but currently ... it does not do that yet.

@victorel-petrovich
Copy link
Author

victorel-petrovich commented Aug 5, 2023

wait a second!
I just noticed that N++ indeed CRASHES when you run that exit() ; meaning, it doesn't recover the changes that you made and didn't save before the crash.
So IT IS a serious problem. Ignoring it means simply not caring about future new users.

I don't understand why you (and others) try to cover it up or defend it, instead of acknowledging, so that a solution will be found, in time.
Hope you understand that I'm not trying to attack PS just for the sake of it.

@victorel-petrovich
Copy link
Author

victorel-petrovich commented Aug 5, 2023

Yes, I understand your point of view. Given how this plugin is implemented and attached to N++, then, yes, it's expected that exit() will crash N++ too.

But I think you can also understand the user's point of view , that exit() should not, not supposed, to kill N++.

So at very least, I think developers should adopt the "primitive" solution above about disabling these commands in startup.py. Untill a more "serious" solution is found.

To date, Python has no mechanism for stopping AND starting runtimes/interpreters. It is initialized (started) and runs until it is terminated (end of process). There are plans to add a functionality called subinterpreter, which would then have to provide such a function, but currently ... it does not do that yet.

I don't fully understand all that, but if you allow me to speculate a little... I'd look at not how Python starts/stops other interpreters, but at how Python itself is started by N++.
If I remember, in general it's possible to specify whether the parent process (or thread) should stop or not when the child process(thread) stops.

@victorel-petrovich victorel-petrovich changed the title [bug?] exit() in a pythonscript causes Notepad++ to exit exit() in a pythonscript causes Notepad++ to CRASH Aug 5, 2023
@victorel-petrovich
Copy link
Author

victorel-petrovich commented Aug 5, 2023

...Or, just don't use exit().

well, yes, that should be one of the first lines in the documentation.

@Ekopalypse
Copy link
Contributor

I don't understand why you (and others) try to cover it up or defend it, instead of acknowledging, so that a solution will be found, in time.

I'm not trying to cover it up or defend it, I was simply stating that the function does what it is supposed to do and therefore it is not a bug. It would be a bug if it didn't do what it was supposed to do.
I honestly don't think it's a good idea to "mother" newbies and try to protect them from all possible dangers. Someone who creates a Python script or downloads it from somewhere and then runs it should be aware of what it is doing. And if the person doesn't, then you should question what you didn't understand.
Documenting this, that exit() and consorts terminate Npp, because no own Python process is started, I advocated.
But this is only my opinion, I may understand that others see it differently and have already expressed this.

I don't fully understand all that, but if you allow me to speculate a little... I'd look at not how Python starts/stops other interpreters, but at how Python itself is started by N++.
If I remember, in general it's possible to specify whether the parent process (or thread) should stop or not when the child process(thread) stops.

No, such dependencies are, to my knowledge, not provided for in the window architecture and would have to be programmed manually. It is actually possible the other way around, i.e. you can start a child process which is not terminated when the parent process ends.
In the case of Python, I tried to explain that there is currently no mechanism that can efficiently start and stop a runtime and this will probably only be possible with the introduction of the mentioned subinterpreters.
Whether then someone can/will actually implement this in PS is, however, a completely different issue.

Untill a more "serious" solution is found.

I don't see that this is as easy to solve as I said at the moment.
In order for exit not to terminate Npp, the Python runtime would have to run in a separate process, which then means that Npp and the respective Python script run in their own address spaces and then communication becomes a bit more difficult. How are you going to synchronize all the pointers?
But again, that's just my opinion ... in the end comes someone who knows what he is doing and just does it ... :-)

@victorel-petrovich
Copy link
Author

victorel-petrovich commented Aug 6, 2023

Documenting this, that exit() and consorts terminate Npp, because no own Python process is started, I advocated.

Thank you.

I don't see that this is as easy to solve as I said at the moment.
In order for exit not to terminate Npp, the Python runtime would have to run in a separate process, which then means that Npp and the respective Python script run in their own address spaces and then communication becomes a bit more difficult. How are you going to synchronize all the pointers?

Good points and questions...

I'll go on a limb here: maybe this could also have advantages, as making it harder to have the kind of dangers we discussed in the other issue (#298) where from PythonScript can change variables of importance outside of Scintilla, or even Notepad++ (if I understood correctly) ?

But again, that's just my opinion ... in the end comes someone who knows what he is doing and just does it ... :-)

So well said... !

@Ekopalypse
Copy link
Contributor

I'll go on a limb here, ...

I don't think so. Concurrent access to a memory area is always the same challenge, whether it's from a thread or a process. You have to make sure that only one can modify the area at a time. Processes only complicate this further because they bring another layer into play, namely the different address spaces. Now you have to deal with, for example, shared memory.

@chcg
Copy link
Collaborator

chcg commented Aug 13, 2023

Seems issue is caused by exception handling at:

after exit(), quit(), sys.exit() triggers the exception boost::python exception:

Ausnahme ausgelöst bei 0x00007FFCC8E9CF19 in notepad++.exe: Microsoft C++-Ausnahme: boost::python::error_already_set bei Speicherort 0x0000004E31BFEC20.

 	PythonScript.dll!boost::python::throw_error_already_set() Zeile 62	C++
>	PythonScript.dll!boost::python::expect_non_null<_object>(_object * x) Zeile 46	C++
 	PythonScript.dll!boost::python::converter::detail::return_object_manager_from_python<boost::python::api::object>::operator()(_object * obj) Zeile 154	C++
 	PythonScript.dll!boost::python::call<boost::python::api::object,boost::python::str>(_object * callable, const boost::python::str & a0, boost::type<boost::python::api::object> * __formal) Zeile 76	C++
 	PythonScript.dll!boost::python::api::object_operators<boost::python::api::object>::operator()<boost::python::str>(const boost::python::str & a0) Zeile 19	C++
 	PythonScript.dll!NppPythonScript::PythonConsole::consume(std::shared_ptr<std::string> statement) Zeile 305	C++
 	PythonScript.dll!NppPythonScript::PyProducerConsumer<std::string>::consumer() Zeile 163	C++
 	PythonScript.dll!NppPythonScript::PyProducerConsumer<std::string>::threadStart(NppPythonScript::PyProducerConsumer<std::string> * instance) Zeile 189	C++

Behaviour could be prevented by avoiding to call PyErr_Print() for that exception type in case the input is directly given to the console.

If the functions are used by a script there is no such exception.
Also os._exit(0) is not causing this and therefore still exits N++.

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

No branches or pull requests

4 participants