DynamoRIO / DynamoRIO/dynamorio

Curiosities during process exit acquire deleted locks, causing a locks_not_closed() assertion failure

Open
#1,058 1 comment 0 reactions 0 assignees View on GitHub
Migrated Priority-Medium
Dominant language
C
Stars
3.2k
Forks
629
Avg merge
2d 15h
Merged PRs (30d)
31

Description

_From [rnk@google.com](https://code.google.com/u/rnk@google.com/) on January 17, 2013 15:00:05_

I'm getting a curiosity from issue #1057 , which is where DR can't open its 1config file for deletion.

I assume that we want to support issuing curiosities at any time, regardless of whether we've already destroyed our global locks. Currently, if we do mutex_delete() and then mutex_lock(), mutex_lock() reinitializes the lock automatically. From report_dynamorio_problem, we end up acquiring a whole bunch of locks, depending on options:

report_buf_lock - for the report
datasec_selfprot_lock - config_reread makes .data writable
dump_core_lock - if dumpcore_mask is set
logdir_mutex - to get the logdir if dumpcore_mask is set
options_lock - to synchronize options
debugbox_lock - if msgbox_mask is set

In theory, we've synched with all threads, so there should be no races. But thread synch success is not guaranteed.

I see a few ways to solve this:
1. Try to make report_dynamorio_problem() lock free. This goes against the grain of the existing code, but gets back to a place where ASSERT and CURIOSITY have few dependencies.
2. Don't acquire locks when issuing reports while exiting. This requires sprinkling ifs through our code, which I don't like.
3. Add a second exit event for lock deletion.
4. Add a bool deleted field to mutex_t and don't count deleted locks in this list in locks_not_closed().

What do you guys think is best?

4 is the smallest change to make exit curiosities work. 1 seems like the ideal, but it's a big change and I'm not sure we want to commit to that.

_Original issue: http://code.google.com/p/dynamorio/issues/detail?id=1058_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.