Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assignment 6 complete #4

Open
wants to merge 17 commits into
base: Assignment-6-base
Choose a base branch
from

Conversation

charan1811
Copy link

No description provided.

Copy link
Contributor

@dwalkes dwalkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
syslog(LOG_DEBUG, "Caught signal, exiting");
caught_sigalarm = true;
closeAll();
sig = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good!

}
total_size += strlen(timestamp);

rc = pthread_mutex_unlock(&writeSocket);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sota6640 you could simplify the logic here and use a single unlock if you save the return value of write and act on it after unlocking, similar to what you do in threadfunc


syslog(LOG_DEBUG, "Write completed to recvfile_fd");
rc = pthread_mutex_lock(&ll_lock);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sota6640 I don't know that the lock is serving any purpose here since you are only accessing the list in a single thread.

==7744== by 0x4866322: allocate_stack (allocatestack.c:622)
==7744== by 0x4866322: pthread_create@@GLIBC_2.2.5 (pthread_create.c:660)
==7744== by 0x10AA74: initTimer (aesdsocket.c:181)
==7744== by 0x10B867: main (aesdsocket.c:569)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an actual issue.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the Valgrind error for the cleanup of timer thread create?

{
int timer_rc;
pthread_t timertid;
if((timer_rc = pthread_create(&timertid, NULL, threadtimerfunc, NULL)) != 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing cleanup of this thread, this is what valgrind is warning you about.

closelog();
syslog(LOG_DEBUG, "PERFORMING CLEANUP1");
//timer_delete(timerid);
pthread_mutex_destroy(&writeSocket);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sota6640 you need to think about whether it's safe to do this cleanup action in each place where the cleanup function is called. writeSocket is one thing which probably isn't safe to destroy in every case you call this function, since your timerThread may still be running.
This is another reason why it's generally better/easier to do cleanup in one place and one code path, much easier to review sequencing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants