Assuming not working properly means it's creating a duplicate contact role? If that's the case, you should consider switching to a map instead of using a list. This way you're not creating duplicates.

I would first build the map of contact roles to opportunities (so a map of a map). We use something like this to create contact roles for opportunities when the opp is created, moved to another account or closed. Something like this:

//Master map, includes all opportunities by contact by contact role: Mapping = Opportunity.Id { Contact.Id { OpportunityContactRole } }
		    map<Id, map<Id, OpportunityContactRole>> existingOpportunitiesContactRolesByContactId = new map<Id, map<Id, OpportunityContactRole>> ();

			//Section checks the opportunities for existing contact roles and adds it to the master map
		    for (Opportunity opportunity : [SELECT Id, AccountId, (SELECT Id, ContactId FROM OpportunityContactRoles) FROM Opportunity WHERE Id IN :opportunities.keySet()]) {	
		    	//Singluar map for one opportunity by contacts by contact roles
		    	map<Id, OpportunityContactRole> existingOpportunityContactRolesByContactId = new map<Id, OpportunityContactRole>();    	
		    	
		    	//Cycle through the opportunity contact roles for this opportunity and assign them to the map
	    		for (OpportunityContactRole opportunityContactRole : opportunity.OpportunityContactRoles)
	    			existingOpportunityContactRolesByContactId.put(opportunityContactRole.ContactId, opportunityContactRole);
		    		
		    	//Updates the master map by adding the opportunity and the corresponding opportunity roles.
		    	existingOpportunitiesContactRolesByContactId.put(opportunity.Id, existingOpportunityContactRolesByContactId);
		    }

Then when you're iterating through your opportunities to determine if the Primary_Contact__c has been updated, then you'll use something like this to pull the list of existing contact roles:
 

for (Opportunity opportunity : opportunities.values()) {
		    	//Pulls all exsiting contact roles from the map for this current opportunity
		    	map<Id, OpportunityContactRole> existingOpportunityContactRolesByContactId = existingOpportunitiesContactRolesByContactId.get(opportunity.Id);

Then something like this to update the existing one if it matches:

Id hasExistingOpportunityContactRole = null;

		        	if(existingOpportunityContactRolesByContactId.containsKey(contact.Id))
		        		hasExistingOpportunityContactRole = existingOpportunityContactRolesByContactId.get(contact.Id).Id;

					//updates master contact roles array for upsert
		            opportunityContactRoles.add( new OpportunityContactRole(Id = hasExistingOpportunityContactRole, Primary = true));

And then you'll upsert on the map like this

if(existingOpportunityContactRolesByContactId != null) {
			    	for(OpportunityContactRole contactRole : existingOpportunityContactRolesByContactId.values()) {

                        if(!contactIdsByAccount.contains(contactRole.ContactId)) 
			        		deleteOpportunityContactRoles.add(contactRole);	
			    	}
			    }

Hopefully I've given you enough examples to make something of it, but using the list is definitely the source of your woes because you're not updating the existing contact role.