Tarantool development patches archive
 help / color / mirror / Atom feed
From: Igor Munkin <imun@tarantool.org>
To: Sergey Ostanevich <sergos@tarantool.org>,
	Alexander Turenko <alexander.turenko@tarantool.org>,
	Oleg Babin <olegrok@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH] gdb: fix the extension to be loaded with Python 2
Date: Sat,  4 Jul 2020 22:58:09 +0300	[thread overview]
Message-ID: <e4aae3261de6faccd91f1229253c63a34d48603e.1593891122.git.imun@tarantool.org> (raw)

There was a mystic error when the extension was loaded against old gdb
versions build against Python 2:
| (gdb) source luajit-gdb.py
| Traceback (most recent call last):
|   File "luajit-gdb.py", line 702, in <module>
|     load(None)
|   File "luajit-gdb.py", line 699, in load
|     'lj-gc': LJGC,
|   File "luajit-gdb.py", line 687, in init
|     command(name)
|   File "luajit-gdb.py", line 468, in __init__
|     gdb.write('{} command initialized\n'.format(name))
| ValueError: sequence.index(x): x not in sequence

I made a little investigation (for more info see the mentioned issue)
and found the next fun fact: the exception was raised much earlier to
<str.format>, more precisely in <gdb.events.new_objfile.disconnect>.
However, the handled exception is preserved until <str.format> call and
hits the condition underneath leading to the extension load failure.

As a result to avoid the exception raise, the special global variable is
introduced for legacy (i.e. Python 2) environment. It checks whether any
callback is associated with new_objfile event prior to disconnecting it.
This variable usage is encapsulated within two introduced routines:
<connect> and <disconnect> which are wrappers for ones provided by gdb.

Furthermore, after diving to gdb sources related to Python embedding, I
found that callbacks are grouped into an internal list. Previous
implementation appended the <load> function to this callback list on
each its unsuccessful call, but only the successful one is removes it
from the list. Thereby disconnect action is moved prior to connect one
so there is no more than one <load> instance kept in callback list.

Fixes tarantool/tarantool#4828

Reported-by: Oleg Babin <olegrok@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
---

Issue: https://github.com/tarantool/tarantool/issues/4828
Branch: imun/gh-4828-luajit-gdb-py2-load

@ChangeLog:
* Fixed the error occurring on loading luajit-gdb.py with Python 2 (gh-4828).

 src/luajit-gdb.py | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index f142fc5..652c560 100644
--- a/src/luajit-gdb.py
+++ b/src/luajit-gdb.py
@@ -7,7 +7,10 @@ import sys
 
 # make script compatible with the ancient Python {{{
 
-if re.match(r'^2\.', sys.version):
+LEGACY = re.match(r'^2\.', sys.version)
+
+if LEGACY:
+    CONNECTED = False
     int = long
     range = xrange
 
@@ -662,19 +665,42 @@ The command requires no args and dumps current GC stats:
 def init(commands):
     global LJ_64, LJ_GC64, LJ_FR2
 
+    # XXX Fragile: though connecting the callback looks like a crap but it
+    # respects both Python 2 and Python 3 (see #4828).
+    def connect(callback):
+        if LEGACY:
+            global CONNECTED
+            CONNECTED = True
+        gdb.events.new_objfile.connect(callback)
+
+    # XXX Fragile: though disconnecting the callback looks like a crap but it
+    # respects both Python 2 and Python 3 (see #4828).
+    def disconnect(callback):
+        if LEGACY:
+            global CONNECTED
+            if not CONNECTED:
+                return
+            CONNECTED = False
+        gdb.events.new_objfile.disconnect(callback)
+
+    try:
+        # Try to remove the callback at first to not append duplicates to
+        # gdb.events.new_objfile internal list.
+        disconnect(load)
+    except:
+        # Callback is not connected.
+        pass
+
     try:
+        # Detect whether libluajit objfile is loaded.
         gdb.parse_and_eval('luaJIT_setmode')
     except:
         gdb.write('luajit-gdb.py initialization is postponed '
                   'until libluajit objfile is loaded\n')
-        gdb.events.new_objfile.connect(load)
+        # Add a callback to be executed when the next objfile is loaded.
+        connect(load)
         return
 
-    try:
-        gdb.events.new_objfile.disconnect(load)
-    except:
-        pass # was not connected
-
     try:
         LJ_64 = str(gdb.parse_and_eval('IRT_PTR')) == 'IRT_P64'
         LJ_FR2 = LJ_GC64 = str(gdb.parse_and_eval('IRT_PGC')) == 'IRT_P64'
-- 
2.25.0

             reply	other threads:[~2020-07-04 20:08 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-04 19:58 Igor Munkin [this message]
2020-07-06  5:50 ` Oleg Babin
2020-07-06 10:24   ` Igor Munkin
2020-07-22 10:46 ` Sergey Ostanevich
2020-07-22 12:00   ` Igor Munkin
2020-07-22 12:46 ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e4aae3261de6faccd91f1229253c63a34d48603e.1593891122.git.imun@tarantool.org \
    --to=imun@tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=olegrok@tarantool.org \
    --cc=sergos@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] gdb: fix the extension to be loaded with Python 2' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox