On 06/12/2009 11:32 PM, Mike Christie wrote:
> On 06/11/2009 03:49 AM, tweyergraf wrote:
>>
>> On Jun 11, 1:29 am, Mike Christie<micha...@cs.wisc.edu> wrote:
>>> On 06/09/2009 09:24 AM, tweyergraf wrote:
>>>
>>> [...]
>>> So after the releasing session entry you get the glibc free error
>>> message, right?
>> Yes, exactly. iscsid crashes precisely in the function session_release
>> in initiator.c. I have wrapped the calls to
>> iscsi_conn_context_free() and free() calles in this function with my
>> own log_debug calls. The call of free(session) *sometimes* causes the
>> crash.
>> Currently, i am about to setup a dedicated test-system to facilitate
>> the possibility of further tests. I think by
>> monday next week, i could do further investigations. If you want me to
>> do something to find out more about this issue, feel free ;)
>> I have used open-iscsi quit intensively and never had such a problem.
>> But this problem occurs on quit a few different systems ( about 10 ).
>> Some are XEN Dom0's some dedicated hosts. They connect to Linux-IET
>> targets as well as to ou EMC storage-backend. The error occurs with
>> both target-types. I am also fairly certain, that we can rule-out
>> networking issus, since the systems are on different networks with
>> different network-setups. Also, the problem arises with the Centos
>> open-iscsi packages, as well as self-build open-iscsi versions 870.3
>> and 871-test4.
>>
>> As stated previously, iscsid runs stable, if I comment-out the two
>> functioncalls above in session_release(). Of course, I am leaking
>> memory in iscsid if I login to new targets ( which fortunatly does not
>> happen often enough).
>>
>> Feel free to request more tests, give suggestions or ask for more
>> information.
>>
>
> Are you doing iscsi boot? In the logs it looks like there are sessions
> running when iscsid starts up. This is fine, iscsid can handle this. But
> maybe if we can take that out of the picture we can narrow the problem down,
>
Could you try the attached patch. It was made against the open-iscsi.git
tree but should apply to 871-test4.
There is a possible double free in the code. I do not think you are
hitting the one I fixed in the patch. It would be really rare to hit it.
I also added some more debug info in there and some more checks though.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~----------~----~----~----~------~----~------~--~---
diff --git a/usr/initiator.c b/usr/initiator.c
index 330e199..9ca6b0f 100644
--- a/usr/initiator.c
+++ b/usr/initiator.c
@@ -81,8 +81,15 @@ static int iscsi_conn_context_alloc(iscsi_conn_t *conn)
ipc->ctldev_bufmax);
if (!conn->context_pool[i]) {
int j;
- for (j = 0; j < i; j++)
+ for (j = 0; j < i; j++) {
free(conn->context_pool[j]);
+ /* tmp hack. in the failure path
+ * iscsi_conn_context_free can be called and
+ * we will double free this if we do not null
+ * it again
+ */
+ conn->context_pool[j] = NULL;
+ }
return ENOMEM;
}
conn->context_pool[i]->conn = conn;
@@ -96,14 +103,17 @@ static void iscsi_conn_context_free(iscsi_conn_t *conn)
int i;
for (i = 0; i < CONTEXT_POOL_MAX; i++) {
- if (!conn->context_pool[i])
+ if (!conn->context_pool[i]) {
+ log_error("BUG trying to call context free again\n");
continue;
+ }
if (conn->context_pool[i]->allocated)
/* missing flush on shutdown */
log_error("BUG: context_pool leak %p",
conn->context_pool[i]);
free(conn->context_pool[i]);
+ conn->context_pool[i] = NULL;
}
}
@@ -488,17 +498,6 @@ __session_conn_create(iscsi_session_t *session, int cid)
return 0;
}
-static void
-session_release(iscsi_session_t *session)
-{
- log_debug(2, "Releasing session %p", session);
-
- if (session->target_alias)
- free(session->target_alias);
- iscsi_conn_context_free(&session->conn[0]);
- free(session);
-}
-
static iscsi_session_t*
__session_create(node_rec_t *rec, struct iscsi_transport *t)
{
@@ -602,9 +601,12 @@ static void
__session_destroy(iscsi_session_t *session)
{
log_debug(1, "destroying session\n");
- list_del(&session->list);
iscsi_flush_context_pool(session);
- session_release(session);
+
+ if (session->target_alias)
+ free(session->target_alias);
+ iscsi_conn_context_free(&session->conn[0]);
+ free(session);
}
static void
@@ -1126,12 +1128,11 @@ void free_initiator(void)
struct iscsi_transport *t;
iscsi_session_t *session, *tmp;
+ log_debug(1, "freeing initiator\n");
+
list_for_each_entry(t, &transports, list) {
- list_for_each_entry_safe(session, tmp, &t->sessions, list) {
- list_del(&session->list);
- iscsi_flush_context_pool(session);
- session_release(session);
- }
+ list_for_each_entry_safe(session, tmp, &t->sessions, list)
+ __session_destroy(session);
}
free_transports();
diff --git a/usr/iscsid.c b/usr/iscsid.c
index ea29bb2..f7c2eb2 100644
--- a/usr/iscsid.c
+++ b/usr/iscsid.c
@@ -278,6 +278,8 @@ static void iscsid_exit(void)
if (daemon_config.initiator_alias)
free(daemon_config.initiator_alias);
free_initiator();
+ idbm_terminate();
+ sysfs_cleanup();
}
static void iscsid_shutdown(void)
@@ -509,7 +511,5 @@ int main(int argc, char *argv[])
isns_fd = isns_init();
event_loop(ipc, control_fd, mgmt_ipc_fd, isns_fd);
iscsid_shutdown();
- idbm_terminate();
- sysfs_cleanup();
return 0;
}
