fix: python debugpy cannot connect #1115
Conversation
debugpy started but not listen on port, so delay some time to wait it initialized. Log: Bug: https://pms.uniontech.com/bug-view-315851.html Change-Id: I14cd10e5b1fcb63a91b38562f9898f71122fbb67
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deepin-mozart The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Reviewer's GuideThis pull request updates the File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @deepin-mozart - I've reviewed your changes - here's some feedback:
- Consider using
QTcpSocket::connectToHostin a loop for a more portable and efficient port check, instead of polling withnetstat. - Consider making the timeout duration (currently 5 seconds) and retry interval for the port check configurable.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| QProcess checkPort; | ||
| QString cmd = QString("netstat -an | grep LISTEN | grep %1").arg(d->port); | ||
| checkPort.start("bash", {"-c", cmd}); | ||
| checkPort.waitForFinished(); |
There was a problem hiding this comment.
suggestion (bug_risk): Add a timeout to waitForFinished().
Specify a timeout for waitForFinished to avoid hanging indefinitely if the netstat command stalls.
| checkPort.waitForFinished(); | |
| if (!checkPort.waitForFinished(500)) { | |
| // If the netstat command stalls, kill the process and continue the retry loop | |
| checkPort.kill(); | |
| qWarning() << "Netstat command timed out for port" << d->port; | |
| QThread::msleep(retryInterval); | |
| continue; | |
| } |
| d->process.setProcessEnvironment(env); | ||
| d->process.start("/bin/bash", options); | ||
| d->process.waitForStarted(); | ||
| QThread::msleep(500); // The port may not start listening immediately when Python starts, resulting in the IDE being unable to connect. Wait for 500ms. |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the polling loop to an asynchronous approach using a QTimer and asynchronous QProcess calls to reduce complexity.
Consider refactoring the polling loop to an asynchronous approach using a centrally managed QTimer and asynchronous QProcess calls. This would simplify the manual retry logic and eliminate spawning a new process on every cycle. For example, you could extract the port check into a dedicated function and use QTimer to periodically trigger the check:
// In your header, add a signal/slot if not already present:
class PythonDebugger : public QObject {
Q_OBJECT
// ...
private slots:
void checkPortReadiness();
private:
QTimer *portCheckTimer = nullptr;
};// In your implementation after starting debugpy:
void PythonDebugger::startDebugpy() {
// Existing process start code ...
// Initialize asynchronous port check
portCheckTimer = new QTimer(this);
portCheckTimer->setInterval(100);
connect(portCheckTimer, &QTimer::timeout, this, &PythonDebugger::checkPortReadiness);
portCheckTimer->start();
}
void PythonDebugger::checkPortReadiness() {
QProcess *checkPort = new QProcess(this);
QString cmd = QString("netstat -an | grep LISTEN | grep %1").arg(d->port);
connect(checkPort, QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished),
[this, checkPort](int, QProcess::ExitStatus) {
if (!checkPort->readAll().isEmpty()) {
portCheckTimer->stop();
qInfo() << "Debugpy port" << d->port << "is ready";
}
checkPort->deleteLater();
});
checkPort->start("bash", {"-c", cmd});
}This approach centralizes the retry logic via a timer and creates a new QProcess only when needed. It keeps functionality intact while reducing overall complexity and unnecessary repeated process creations.
2 participants
debugpy started but not listen on port, so delay some time to wait it initialized.
Log:
Bug: https://pms.uniontech.com/bug-view-315851.html Change-Id: I14cd10e5b1fcb63a91b38562f9898f71122fbb67
Summary by Sourcery
Improve debugpy port initialization by implementing a more robust port readiness check mechanism
Bug Fixes:
Enhancements: