Home page logo

nmap-dev logo Nmap Development mailing list archives

Re: [NSE] Resource Cleanup Upon Thread Death
From: David Fifield <david () bamsoftware com>
Date: Tue, 2 Jun 2009 18:41:54 -0600

On Tue, Jun 02, 2009 at 05:37:08PM -0600, Patrick Donnelly wrote:
On Tue, Jun 2, 2009 at 4:41 PM, David Fifield <david () bamsoftware com> wrote:
I think this patch is good and should be committed, but first I ahve
just a few questions about the code.

I am having trouble keeping track of the upvalue indices associated with
a thread. I would like you to add a big block comment describing what
each index contains.

Alright, I've added that.

+  function Thread:close ()
+    local ch = self.close_handlers;
+    for i = #ch, 1, -1 do
+      ch[i].destructor(ch[i].thread, i);
+      ch[i] = nil;
+    end
+    for key, destructor_t in pairs(ch) do
+      destructor_t.destructor(destructor_t.thread, key);
+      ch[key] = nil;
+    end
+  end

Please explain what this bit of code is doing and why there are two
loops. It looks like you are calling all the integer-keyed destructors
in reverse order, then all the destructors with other keys in whatever
order they appear. What is the structure of close_handlers? Is it
possible to enforce integer keys only and simply this?

This is a remnant of something else I was doing and I have removed the
integer key part. The structure is simply unique destructor keys to a
table containing two fields (you can see these tables made in the
definition of _R[DESTRUCTOR] in nse_main.lua).

@@ -396,6 +412,7 @@
   -- nse_restore, waiting threads become pending and later are moved all
   -- at once back to running.
   local running, waiting, pending = {}, {}, {}
+  local all = setmetatable({}, {__mode = "kv"}); -- base coroutine to Thread
   -- hosts maps a host to a list of threads for that host.
   local hosts, total = {}, 0
   local current
@@ -406,19 +423,37 @@
     local thread = remove(threads);
     thread:d("Starting %THREAD against %s%s.", thread.host.ip,
         thread.port and ":"..thread.port.number or "");
-    running[thread.co], total = thread, total + 1;
+    all[thread.co], running[thread.co], total = thread, thread, total+1;
     hosts[thread.host] = hosts[thread.host] or {};
     hosts[thread.host][thread.co] = true;

Is the new "all" table to facilitate your planned change to allow a
script thread to start its own coroutines?

This all table simply maps the original thread (coroutine) to the
Thread Class it belongs to. I removed the changes (also remnants of
other things I was working on) to WAITING_TO_RUNNING, which probably
made it more confusing.. We need the all table to obtain the
coroutine's Thread (Class). The coroutine we are setting a destructor
on may not be running (if it were, we could use the 'current'

Okay, thanks, please commit this new patch.

David Fifield

Sent through the nmap-dev mailing list
Archived at http://SecLists.Org

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]