Opened 8 years ago

Closed 8 years ago

#1275 closed defect (fixed)

A temporary socket file is not removed after each statistics test

Reported by: naokikambe Owned by: naokikambe
Priority: medium Milestone: Sprint-20111025
Component: statistics Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 3 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

After #1175, such a temporary socket file is created and used in each statistics test, but it is there even after each test is done. It should be removed after each test.

[kambe@fdvm ~]$ rm -f /tmp/msgq_socket_*
[kambe@fdvm ~]$ ls -ltr /tmp/msgq_socket_*
ls: cannot access /tmp/msgq_socket_*: No such file or directory
[kambe@fdvm ~]$ cd git/src/bin/stats
[kambe@fdvm stats]$ make check
....
[kambe@fdvm stats]$ ls -ltr /tmp/msgq_socket_*
srwxrwxr-x 1 kambe kambe 0 Oct  4 09:57 /tmp/msgq_socket_uSWIia
srwxrwxr-x 1 kambe kambe 0 Oct  4 09:57 /tmp/msgq_socket_DkyT71
[kambe@fdvm stats]$ make check
....
[kambe@fdvm stats]$ ls -ltr /tmp/msgq_socket_*
srwxrwxr-x 1 kambe kambe 0 Oct  4 09:57 /tmp/msgq_socket_uSWIia
srwxrwxr-x 1 kambe kambe 0 Oct  4 09:57 /tmp/msgq_socket_DkyT71
srwxrwxr-x 1 kambe kambe 0 Oct  4 09:57 /tmp/msgq_socket_0E_Hwb
srwxrwxr-x 1 kambe kambe 0 Oct  4 09:57 /tmp/msgq_socket_NUGzQc
[kambe@fdvm stats]$

Subtickets

Change History (10)

comment:1 Changed 8 years ago by naokikambe

  • Owner set to naokikambe
  • Status changed from new to accepted

comment:2 Changed 8 years ago by jreed

And we should use the builddir instead of /tmp/.

comment:3 Changed 8 years ago by naokikambe

  • Owner changed from naokikambe to UnAssigned
  • Status changed from accepted to reviewing

Yes, I moved the dir. And the branch is ready for reviewing.

comment:4 Changed 8 years ago by naokikambe

  • Estimated Difficulty changed from 0 to 3

comment:5 Changed 8 years ago by shane

  • Milestone changed from New Tasks to Next-Sprint-Proposed

comment:6 Changed 8 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20111025

comment:7 follow-up: Changed 8 years ago by zhanglikun

  • Owner changed from UnAssigned to naokikambe

The following comments come from Terry(a new CNNIC developer for bind10), since he doesn't have a wiki account, so use my account first.

I have already review the changed codes. A good job!

But I have a question to the following change:

class SignalHandler():
     """A signal handler class for deadlock in unittest"""
     def __init__(self, fail_handler, timeout=20):
@@ -105,14 +101,9 @@ class MockMsgq:
             self.msgq.run()
         except Exception:
             pass
-        finally:
-            # explicitly shut down the socket of the msgq before
-            # shutting down the msgq
-            self.msgq.listen_socket.shutdown(msgq.socket.SHUT_RDWR)
-            self.msgq.shutdown()
 
     def shutdown(self):
-        # do nothing for avoiding shutting down the msgq twice
+        # do nothing
         pass

delete the finally branch.

"In real world applications, the finally clause is useful for releasing external resources (such as files or network connections), regardless of whether the use of the resource was successful."

I am not sure why delete 'finally' branch here?

comment:8 in reply to: ↑ 7 Changed 8 years ago by naokikambe

  • Owner changed from naokikambe to zhanglikun

Hello Terry-san,

Thank you for reviewing.

Replying to zhanglikun:

"In real world applications, the finally clause is useful for releasing external resources (such as files or network connections), regardless of whether the use of the resource was successful."

I didn't consider such a situation enough. I restored that. Please see git 8b024ff407.

comment:9 Changed 8 years ago by zhanglikun

  • Owner changed from zhanglikun to naokikambe

Hi, Kambe

I have already reviewed the changed codes. All seem good. In my opinion,I think #1275 can merge into the master.

For my wiki's account(Kevin_tes) limits of authority to modify the ticket,forgive me not feedback to you in time。

comment:10 Changed 8 years ago by naokikambe

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merging has been done and then I'm closing.
And also I suggest you to ask Jeremy to modify your account:)
Thanks,

Note: See TracTickets for help on using tickets.