[pygame] PATCH: pyportmidi object de-allocation error (and fix)

[Christopher Arndt <chris@xxxxxxxxxxxxx>]

Hi all,

this has been bugging me for some time but now I finally found the time
to look into this thoroughly: each time when using pygame.midi on
Ubuntu, I got a BadPointer error when closing an Input or Output
instance or when the program ends:

    >>> from pygame import midi
    >>> midi.init()
    >>> i = midi.Input(1)
    >>> i.close()
    PortMidi call failed...
      PortMidi: `Bad pointer'
    type ENTER...

This was very annoying since apparently the Ubuntu version of
libportmidi is a debug build, which will terminate the program with an
error message whenever an error occurs in a portmidi call.

So I compiled portmidi form source (SVN rev 222) and then pygame from
SVN as well (I had to comment out the PORTTIME dependency in
config_unix.py, since porttime is compiled into portmidi by default, but
that is another problem.)

But I still got the BadPointer errorr, only this time the exception was
ignored. So I looked at pygame/lib/midi.py and pygame/src/pypm.pyx and
found out what's wrong:

<important part>
pygame.midi.Input.close() calls pypm.Input.Close() and then sets
self._input = None. This causes the pypm.Input instance, which was
referenced by this attribute, to be garbage collected, which in turn
causes pypm.Input.__dealloc__() to be called. This then tries to close
the input device again unconditionally and this is where the error
occurs since the pointer to the device is not valid any more, of course.

The same error is presents in pygame.midi.Output / pypm.Output.
</important part>

I attached a minimal small patch which should fix this issue.
Interestingly, part of the fix was already present in the code but
commented out. After applying the patch (you may have to use the '-l'
option with patch) pygame/src/pypm.c has to be rebuilt with Pyrex/Cython.

Note: since I found that the pypm.pyx source is really a mess (source
code formatting and other coding-style issues and also some programming
inconsistencies) I did a bigger scope code cleanup, for which I have
prepared a separate patch. I not sure which is the right place to submit
this. Should I post this here or should I file a bug at
https://bitbucket.org/aalex/pyportmidi ?


Chris


Index: src/pypm.pyx
===================================================================
--- src/pypm.pyx	(Revision 3006)
+++ src/pypm.pyx	(Arbeitskopie)
@@ -238,13 +238,15 @@
                         print "Unable to open Midi OutputDevice=",OutputDevice," err=",s

     def __dealloc__(self):
-        if self.debug: print "Closing MIDI output stream and destroying instance"
-        #err = Pm_Abort(self.midi)
-        #if err < 0: raise Exception, Pm_GetErrorText(err)
-        err = Pm_Close(self.midi)
-        if err < 0: raise Exception, Pm_GetErrorText(err)
+        if self.debug:
+            print "Closing MIDI output stream and destroying instance"

+        if self.midi:
+            err = Pm_Close(self.midi)
+            if err < 0:
+                raise Exception, Pm_GetErrorText(err)

+
     def _check_open(self):
         """ checks to see if the midi is open, and if not, raises an error.
         """
@@ -262,15 +264,16 @@
     (PortMidi attempts to close open streams when the application
     exits -- this is particularly difficult under Windows.)
         """
-        #if not self.midi:
-        #    return
+        if not self.midi:
+            return

         err = Pm_Close(self.midi)
         if err < 0:
             raise Exception, Pm_GetErrorText(err)
-        #self.midi = NULL

+        self.midi = NULL

+
     def Abort(self):
         """
 Abort() terminates outgoing messages immediately
@@ -280,8 +283,8 @@
     ignore messages in the buffer and close an input device at
     any time.
         """
-        #if not self.midi:
-        #    return
+        if not self.midi:
+            return

         err = Pm_Abort(self.midi)
         if err < 0:
@@ -411,14 +414,15 @@

     def __dealloc__(self):
         cdef PmError err
-        if self.debug: print "Closing MIDI input stream and destroying instance"
+        if self.debug:
+            print "Closing MIDI input stream and destroying instance"

-        err = Pm_Close(self.midi)
-        if err < 0:
-            raise Exception, Pm_GetErrorText(err)
+        if self.midi:
+            err = Pm_Close(self.midi)
+            if err < 0:
+                raise Exception, Pm_GetErrorText(err)


-
     def _check_open(self):
         """ checks to see if the midi is open, and if not, raises an error.
         """
@@ -434,14 +438,15 @@
     (PortMidi attempts to close open streams when the application
     exits -- this is particularly difficult under Windows.)
         """
-        #if not self.midi:
-        #    return
+        if not self.midi:
+            return

-        err = Pm_Close(self.midi)
-        if err < 0:
-            raise Exception, Pm_GetErrorText(err)
-        #self.midi = NULL
+        if self.midi:
+            err = Pm_Close(self.midi)
+            if err < 0:
+                raise Exception, Pm_GetErrorText(err)

+        self.midi = NULL


     def SetFilter(self, filters):