[gdb/python] Avoid queue.SimpleQueue for python 3.6
Commit Message
[ was: Re: [PATCH] Initial implementation of Debugger Adapter Protocol ]
On 1/4/23 13:27, Tom de Vries wrote:
> On 1/4/23 12:59, Tom de Vries via Gdb-patches wrote:
>> On 1/3/23 15:14, Tom Tromey wrote:
>>> Tom> /home/vries/gdb_versions/devel/src/gdb/python/py-dap.c: In member
>>> Tom> function ‘virtual void dap_interp::init(bool)’:
>>> Tom> /home/vries/gdb_versions/devel/src/gdb/python/py-dap.c:83:27:
>>> error:
>>> Tom> ‘PyObject_CallNoArgs’ was not declared in this scope
>>> Tom> gdbpy_ref<> result_obj (PyObject_CallNoArgs (func.get ()));
>>> Tom> ^~~~~~~~~~~~~~~~~~~
>>> Tom> /home/vries/gdb_versions/devel/src/gdb/python/py-dap.c:83:27: note:
>>> Tom> suggested alternative: ‘_PyObject_CallNoArg’
>>> Tom> gdbpy_ref<> result_obj (PyObject_CallNoArgs (func.get ()));
>>> Tom> ^~~~~~~~~~~~~~~~~~~
>>> Tom> _PyObject_CallNoArg
>>> Tom> ...
>>>
>>> Sorry about that. I'm going to apply the appended, which should fix the
>>> problem.
>>
>> That fixed the build problem, thanks for that.
>>
>> Now I'm running into an error in the testsuite:
>> ...
>> ERROR: eof reading json header
>> while executing
>> "error "eof reading json header""
>> invoked from within
>> "expect {
>> -i exp19 -timeout 10
>> -re "^Content-Length: (\[0-9\]+)\r\n" {
>> set length $expect_out(1,string)
>> exp_continue
>> }
>> -re "^(\[^\r\n\]+)..."
>> ("uplevel" body line 1)
>> invoked from within
>> "uplevel $body" NONE eof reading json header
>> UNRESOLVED: gdb.dap/basic-dap.exp: startup - initialize
>> ...
>
> Hmm, that seems to be because:
> ...
> (gdb)
> #5 0x0000000000a59a7c in gdbpy_handle_exception ()
> at /home/vries/gdb_versions/devel/src/gdb/python/py-utils.c:396
> 396 error (_("Error occurred in Python: %s"), msg.get ());
> (gdb) p msg.get ()
> $1 = 0x2b91d10 "module 'queue' has no attribute 'SimpleQueue'"
> ...
>
> That's new in python 3.7, and I have:
> ...
> $ ldd ./gdb | grep python
> libpython3.6m.so.1.0 => /usr/lib64/libpython3.6m.so.1.0
> (0x00007f93ed624000)
> ...
Fixed by attached commit, any comments?
Thanks,
- Tom
Comments
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> Fixed by attached commit, any comments?
Thank you for working on this. I saw your note yesterday but was too
busy to address it.
Tom> + if sys.version_info[0] == 3 and sys.version_info[1] <= 6:
Tom> + self.write_queue = queue.Queue()
Tom> + else:
Tom> + self.write_queue = queue.SimpleQueue()
It wasn't clear to me what happens with Queue if task_done is never
called. Like, does it "leak" memory? This is why I chose SimpleQueue.
Maybe we can just switch to Queue entirely and have the popping code
also call task_done immediately. That seems ok since we don't use join.
I guess what I would do here is have a helper module that uses
SimpleQueue if it is available, and otherwise implements a look-alike
using Queue.
I don't know if I can work on this today but I will get to it soon,
unless you feel like trying it out.
If Queue can be dropped in like your patch without any negative effects,
then that's also totally fine by me.
Tom
On 1/5/23 16:19, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>
> Tom> Fixed by attached commit, any comments?
>
> Thank you for working on this. I saw your note yesterday but was too
> busy to address it.
>
> Tom> + if sys.version_info[0] == 3 and sys.version_info[1] <= 6:
> Tom> + self.write_queue = queue.Queue()
> Tom> + else:
> Tom> + self.write_queue = queue.SimpleQueue()
>
> It wasn't clear to me what happens with Queue if task_done is never
> called. Like, does it "leak" memory? This is why I chose SimpleQueue.
>
> Maybe we can just switch to Queue entirely and have the popping code
> also call task_done immediately. That seems ok since we don't use join.
> I guess what I would do here is have a helper module that uses
> SimpleQueue if it is available, and otherwise implements a look-alike
> using Queue.
>
> I don't know if I can work on this today but I will get to it soon,
> unless you feel like trying it out.
>
> If Queue can be dropped in like your patch without any negative effects,
> then that's also totally fine by me.
I agree that the semantics as advertised for task_done are a bit vague.
I read about it a bit here (
https://stackoverflow.com/questions/49637086/python-what-is-queue-task-done-used-for
) to understand what it's supposed to do and looked at the
implementation here (
https://github.com/python/cpython/blob/master/Lib/asyncio/queues.py ),
and by now I'm convinced that there's no memory leak.
So I'll commit shortly.
Thanks,
- Tom
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> I agree that the semantics as advertised for task_done are a bit vague.
Tom> I read about it a bit here (
Tom> https://stackoverflow.com/questions/49637086/python-what-is-queue-task-done-used-for
Tom> ) to understand what it's supposed to do and looked at the
Tom> implementation here (
Tom> https://github.com/python/cpython/blob/master/Lib/asyncio/queues.py ),
Tom> and by now I'm convinced that there's no memory leak.
Tom> So I'll commit shortly.
Thanks for doing this.
Tom
From 2704ba927c369d834d8b88adb136e323f3c2036d Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Thu, 5 Jan 2023 10:34:58 +0100
Subject: [PATCH] [gdb/python] Avoid queue.SimpleQueue for python 3.6
On openSUSE Leap 15.4 with python 3.6, the gdb.dap/basic-dap.exp test-case
fails as follows:
...
ERROR: eof reading json header
while executing
"error "eof reading json header""
invoked from within
"expect {
-i exp19 -timeout 10
-re "^Content-Length: (\[0-9\]+)\r\n" {
set length $expect_out(1,string)
exp_continue
}
-re "^(\[^\r\n\]+)..."
("uplevel" body line 1)
invoked from within
"uplevel $body" NONE eof reading json header
UNRESOLVED: gdb.dap/basic-dap.exp: startup - initialize
...
Investigation using a "catch throw" shows that:
...
(gdb)
at gdb/python/py-utils.c:396
396 error (_("Error occurred in Python: %s"), msg.get ());
(gdb) p msg.get ()
$1 = 0x2b91d10 "module 'queue' has no attribute 'SimpleQueue'"
...
The python class queue.SimpleQueue was introduced in python 3.7.
Fix this by falling back to queue.Queue for python <= 3.6.
Tested on x86_64-linux, by successfully running the test-case:
...
# of expected passes 47
...
---
gdb/python/lib/gdb/dap/server.py | 6 +++++-
gdb/python/lib/gdb/dap/startup.py | 6 +++++-
2 files changed, 10 insertions(+), 2 deletions(-)
@@ -15,6 +15,7 @@
import json
import queue
+import sys
from .io import start_json_writer, read_json
from .startup import (
@@ -47,7 +48,10 @@ class Server:
# This queue accepts JSON objects that are then sent to the
# DAP client. Writing is done in a separate thread to avoid
# blocking the read loop.
- self.write_queue = queue.SimpleQueue()
+ if sys.version_info[0] == 3 and sys.version_info[1] <= 6:
+ self.write_queue = queue.Queue()
+ else:
+ self.write_queue = queue.SimpleQueue()
self.done = False
global _server
_server = self
@@ -22,6 +22,7 @@ import signal
import threading
import traceback
from contextlib import contextmanager
+import sys
# The GDB thread, aka the main thread.
@@ -173,7 +174,10 @@ def send_gdb_with_response(fn):
"""
if isinstance(fn, str):
fn = Invoker(fn)
- result_q = queue.SimpleQueue()
+ if sys.version_info[0] == 3 and sys.version_info[1] <= 6:
+ result_q = queue.Queue()
+ else:
+ result_q = queue.SimpleQueue()
def message():
try:
base-commit: b26c8438c71d44427fc9c7ef6fa2ab1742d220a9
--
2.35.3