python-pillow / Sane

Python interface to the SANE scanner and frame grabber
Other
54 stars 19 forks source link

Not closing device but calling sane.exit causes segfault on exit #22

Closed bodograumann closed 8 years ago

bodograumann commented 8 years ago

I forgot to close the SaneDev that I opened, but I did issue a sane.exit. This caused a segfault when exiting python. E.g.:

Python 3.4.3 (default, Jan 29 2016, 15:13:46) 
[GCC 4.9.3] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sane
>>> sane.init()
(16777240, 1, 0, 24)
>>> dev = sane.get_devices()[0][0]
>>> sane.open(dev)
<sane.SaneDev object at 0x7f17a14950f0>
>>> sane.exit()
>>> exit()
zsh: segmentation fault  python

The reason is, that when the SaneDev is destroyed on exit, sane_close is called implicitly, even if sane_exit was already called.

I don’t know if it should be valid to skip sane_close, as the documentation only says:

When done using the device, the handle should be closed by a call to sane_close().

In any case, I think that sane.exit should complain about any open devices, because it does invalidate the SaneDev instances.

Aside, the documentation goes on to say:

Finally, before exiting the application, function sane_exit() must be called. It is important not to forget to call this function since otherwise some resources (e.g., temporary files or locks) may remain unclaimed.

So sane.exit should probably also be called implicitly on exit.

bodograumann commented 8 years ago

oh, and here is the documentation link (“Code Flow”): http://www.sane-project.org/html/doc013.html

manisandro commented 8 years ago

Not sure I'm covinced by this. The question is always where to draw the line between trying to prevent users to shoot themselves in the foot and simple code. From my point of view the code behaves correctly and works as documented (true, one can always argue that "should" is not equal to "must", however from my experience what in an API documentation is written as "should" almost always effectively means "must"). If you feel strongly about this however, feel free to open a pull request with a proposal.

bodograumann commented 8 years ago

I agree that most of what I suggested is debatable and it’s a question of project policy. But I think the actual cause of the segfault is a true bug. I.e. sane_close should not implicitly be called if sane_exit was called before. If we assume the user properly uses the sequence

there would be no need for an implicit SaneDev.close.

manisandro commented 8 years ago

So basically something like https://github.com/python-pillow/Sane/commit/37cd9f78cc78c362db1f48028c848ab8954cbdf8 (untested)?

bodograumann commented 8 years ago

Yes, this works for me.

manisandro commented 8 years ago

Committed, thanks for testing.